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

Remove widget states from notebooks #256

Merged
merged 1 commit into from
Jun 10, 2023
Merged

Conversation

IAlibay
Copy link
Member

@IAlibay IAlibay commented Jun 4, 2023

I've gone ahead and cleared all the widget states from all the notebooks under examples. This reduces the total size of the doc files from 154 MB to 60 MB.

As far as I'm aware this shouldn't affect how the notebooks render, but someone please double check the RTD results.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@pgbarletta
Copy link
Contributor

I did the same when I added the nbval testing. They seem to accumulate with each run.

@IAlibay
Copy link
Member Author

IAlibay commented Jun 9, 2023

I did the same when I added the nbval testing. They seem to accumulate with each run.

they shouldn't be getting committed, I think it's more of a case that someone forgot to clear them after an update

@lilyminium
Copy link
Member

I think this removes the ability to look interactively at NGLView widgets on rendered docs. However I think the benefits of keeping the interactivity is much outweighed by the costs of storage compared to, say, a gif.

@lilyminium
Copy link
Member

And the huge diffs are a pain.

Copy link
Member

@lilyminium lilyminium left a comment

Choose a reason for hiding this comment

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

LGTM. Not sure how this will interact with #251 so I'll let you merge.

@lilyminium
Copy link
Member

Actually I'll merge now so I can build off it and replace widgets with gifs.

@lilyminium lilyminium merged commit cd0a688 into develop Jun 10, 2023
@lilyminium lilyminium deleted the clear-widgetstate branch June 10, 2023 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants