Add --remove-all-notebook-metadata flag

[yasirroni]

Add new argument for clean and filter "-M" or "--remove-notebook-metadata" to "remove notebook metadata.

    clean_parser.add_argument(
        "-M",
        "--remove-notebook-metadata",
        action="store_true",
        help="remove notebook metadata",
    )

[codecov]

Codecov Report

Base: 98.07% // Head: 97.02% // Decreases project coverage by -1.05% ⚠️

Coverage data is based on head (b07f6d6) compared to base (5ddf4f7).
Patch coverage: 86.66% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #163      +/-   ##
==========================================
- Coverage   98.07%   97.02%   -1.06%     
==========================================
  Files           3        3              
  Lines         156      168      +12     
==========================================
+ Hits          153      163      +10     
- Misses          3        5       +2     

Impacted Files	Coverage Δ
src/nb_clean/__init__.py	97.67% <84.61%> (-2.33%)	⬇️
src/nb_clean/cli.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[srstevenson]

As this is branched from #161, I'll wait until that's complete before reviewing this.

[yasirroni]

#161 is merged. Please review this branch. I will rebase this branch.

[yasirroni]

After some reading it seems current behavior of nb-clean is cleaning notebook metadata?

[srstevenson]

Before we consider merging this, we need to think more about the approach and how to maintain compatibility for existing users.

With this change, there are --preserve-notebook-metadata and --remove-notebook-metadata arguments which sound as though they are the inverse of one another but do different things and are toggled independently. This is confusing for users, and also for me to maintain.

--preserve-notebook-metadata only checks for and removes the language_info.version key from the metadata, whereas the new --remove-notebook-metadata argument removes all metadata keys. We need the names to disambiguate this: for example, by renaming --remove-notebook-metadata to --remove-all-notebook-metadata or similar.

Even if it were named differently, I'm not keen to maintain two flags so easily confused with each other. Perhaps we should deprecate --preserve-notebook-metadata? This can be achieved in a sequence of steps across future releases:

• from this change, using --preserve-notebook-metadata prints a deprecation warning that the feature will be removed in future
• after the deprecation warning has been present for some time, the flag becomes a no-op that prints a warning saying the feature was removed
• eventually, we remove the flag entirely and passing it will result in an argument parsing error

In addition, please can you run the linters and test suite with poetry run poe check and address the issues raised?

[yasirroni]

All updated. Please review.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.51%. Comparing base (b71adf0) to head (1432d1b).
Report is 25 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #163      +/-   ##
==========================================
+ Coverage   99.46%   99.51%   +0.04%     
==========================================
  Files           3        3              
  Lines         187      205      +18     
==========================================
+ Hits          186      204      +18     
  Misses          1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[srstevenson]

Thanks, I've made another round of review -- we're making good progress.

[yasirroni]

All resolved with 100% coverage. Please review.

[srstevenson]

LGTM, thanks for perservering with this.

[srstevenson]

This change is included in the nb-clean 3.3.0 release.