Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support only stripping outputs larger than a given size #135

Merged
merged 2 commits into from
May 5, 2021

Conversation

cjblocker
Copy link
Contributor

I need to be able to strip embedded images and videos from a notebook while leaving the smaller text output (similar to #58). This PR adds a --max-size option which removes output larger than max-size bytes.

$ nbstripout --max-size 5K test1.ipynb
$ nbstripout --max-size 100 test2.ipynb
$ nbstripout --max-size 1M test3.ipynb

This easily delineates larger binary outputs from smaller textual ones.

I did not want to just filter on keys (an alternative) because a matplotlib animation video may be embedded in a text/html key, but I don't want to remove every text/html key. I'm also not sure if embedded images are always image/png in all kernels and plotting backends.

I've run some ad-hoc benchmarks and computing the size of the outputs in negligible to the overall run time when enabled (~2%-4%).

Thanks!

Copy link
Owner

@kynan kynan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! This is looking pretty good. Some suggested simplifications.

Please rebase and add a test as well!

nbstripout/_utils.py Outdated Show resolved Hide resolved
nbstripout/_utils.py Outdated Show resolved Hide resolved
@cjblocker
Copy link
Contributor Author

@kynan Great, I have added your suggestions. Let me know if there is anything else!

Copy link
Owner

@kynan kynan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's good to go now :)

@kynan kynan merged commit 0b83f9a into kynan:master May 5, 2021
@kynan kynan added resolution:merged and removed state:waiting Waiting for response for reporter labels May 5, 2021
@kynan kynan added this to the 0.5.0 milestone May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants