Resizing images and videos in a jupyter notebook

[kolibril13]

I was now working a while with manim in jupyter notebook, and the biggest issue I see is the inconsistency with the output width of videos/images. Currently, one can only scale videos, and also only in advance, not afterwards.
Images can be only scaled by using the quality flags, but in a notebook with let's say 20 scenes, this causes a mix of large and small cells, which don't look very organized.
This pr will enable three options, which are in my opinion very useful: 1. a thumbnail view that does not take a lot of space 2. a larger view where one can see the scene nicely, 3. A view where the video/image is displayed pixel by pixel. All options can be also selected after the video is rendered.
Additionally, I added a Download button for the images.

EDIT: tests are failing, can someone tell me how to add ipywidgets as a dependency to the repo?
https://user-images.githubusercontent.com/44469195/114287021-fc48c780-9a63-11eb-8d35-ef2695e2f0e0.mov

Testing Status

Further Comments

Checklist

• I have read the Contributing Guidelines
• I have written a descriptive PR title (see top of PR template for examples)
• I have written a changelog entry for the PR or deem it unnecessary
• My new functions/classes either have a docstring or are private
• My new functions/classes have tests added and (optional) examples in the docs
• My new documentation builds, looks correctly formatted, and adds no additional build warnings

Reviewer Checklist

• The PR title is descriptive enough
• The PR is labeled correctly
• The changelog entry is completed if necessary
• Newly added functions/classes either have a docstring or are private
• Newly added functions/classes have tests added and (optional) examples in the docs
• Newly added documentation builds, looks correctly formatted, and adds no additional build warnings

[Darylgolden]

I made a PR to your branch, updating the dependencies in poetry which should hopefully fix the tests.

[Darylgolden]

I also note that the icons do not show up correctly on Colab (but do on my local installation).

Other than that, LGTM!

[Darylgolden]

Is it also possible to use relative heights and widths instead of pixel values?

[kolibril13]

I also note that the icons do not show up correctly on Colab (but do on my local installation).

that seems to be a known issue:
googlecolab/colabtools#1302
I'd say don't try to solve this issue in this pr, but keep it for the future (maybe it will be solved by google by then :)).
I've now updated the poetry dependencies just as you suggested.

Is it also possible to use relative heights and widths instead of pixel values?

I think the height should always be defined by the video shape and only with width should change, otherwise, the video would look distorted. And for the widths, I prefer absolute pixel values over relative pixel values.

[kolibril13]

@naveen521kk : Is it ok when the optional parameter is false?
I don't know why this happend

[naveen521kk]

if you ran poetry lock or poetry update and this happened it should be fine.

[kolibril13]

@Darylgolden : Did you run poetry lock or poetry update ?

[Darylgolden]

I did poetry add.

[jsonvillanueva]

This is also fine, but in particular since this is a dependency for jupyter-lab/Colab, it should be placed in the extras (see L57 of pyproject.toml)

[kolibril13]

Yep, that is possible, but I think it is also ok to be in the main dependencies, no?
I am thinking of the case that someone had already installed jupyter notebook and then installs manim, but has no jupyter lab, it would be nice when this cell magic works out of the box. Or are the extras automatically installed with pip install manim? If it would work automatically, then we can add it to the extras.
Further, I have a lot of extra installed packages in my manim environment, and I am afraight that when I type poetry update that these will be also added to the poetry file.

[jsonvillanueva]

No need to worry about extra installed packages in your manim environment. poetry update will only update the versions of the packages listed in pyproject.toml.

The extras are not automatically installed via pip, nor are they by poetry.
In either package manager, you must specify the extra e.g.:
poetry install -E jupyterlab
pip install manim[jupyterlab]

Since jupyterlab is not a requirement and is already part of the extras, its dependencies should also be extras.

[kolibril13]

@jsonvillanueva : Thanks for updating the poetry file.
Unfortunately, the tests are now failing.

=========================== short test summary info ============================
ERROR manim/utils/ipython_magic.py - ModuleNotFoundError: No module named 'ip...
!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
=============================== 1 error in 4.83s ===============================

Can you have a look at that?

[kolibril13]

Now the issue here seems to be hardcoded defaults with the ipywidgets... and looking at ipywidgets:widget_media.py, their Video object doesn't appear to support responsive width. So it can't make use of config.media_width="50vw", or 50%, but it still works for say config.media_width="782px". It's not a major flaw, but I think that on 4k resolutions this will be problematic with 30px buttons.

I think absolute pixel values are fine for now, but I am totally fine if someone wants to add relative values in a future pr.
Regarding the issue with 30px on a 4k screen: I have a WQHD screen, and they look nice in size, so I would not make them bigger. When people with 4k screens find them too small, the code would be small as well, no?
I would not be too concerned about it, but this can also be iterated when it would become a problem

[jsonvillanueva]

I think @behackl will have better feedback on this front of 4k (I don't own a 4k monitor).

As for the config.media_width fields, it's still plausible to do in this PR as it's really only a couple lines of code that need to be changed; however, I'm not sure if it's good to break the behavior that @Darylgolden just added in the previous PR related to video sizes where dynamic sizing WAS supported... Also it seems like with this implementation there would need to be new config variables (particularly for config.min_media_width and config.max_media_width) based on the new toggle buttons.

[Darylgolden]

Personally, I think there should be a media_min_width, media_max_width and expanded boolean (to track whether the media should be expanded or not. I think the default media_max_width should be 100%, and the default expanded can be up to a poll (since I think many users, including myself, prefer the expanded view). I think relative values should be used but they can be added in a future PR.

[Darylgolden]

Hi, are you still interested in working on this? If not, I think we can merge this as is and make the changes in future PRs.

[kolibril13]

Hi, are you still interested in working on this? If not, I think we can merge this as is and make the changes in future PRs.

Right now, I have other priorities.
And one thing I've noted when working with this:
When restarting the notebook, all cell outputs are converted into strings, which makes it impossible to see the images:
jupyter-widgets/ipywidgets#3183
That was possible with the current implementation.
I think this should also be addressed in a future pr, as a preview of previous work is quite useful.
I had e.g. the idea of a function that can be called before closing the notebook, which converts all the cells into static images/videos like it is the behaviour before this pr.

[Darylgolden]

@jsonvillanueva Can we merge this PR as is? The relative sizing and use of the global config can be worked on later.

[jsonvillanueva]

Sure; just handle the poetry conflicts @kolibril13

[kolibril13]

Sure; just handle the poetry conflicts @kolibril13

Resolved.
But there is still this issue:

And one thing I've noted when working with this:
When restarting the notebook, all cell outputs are converted into strings, which makes it impossible to see the images:
jupyter-widgets/ipywidgets#3183
That was possible with the current implementation.
I think this should also be addressed in a future pr, as a preview of previous work is quite useful.
I had e.g. the idea of a function that can be called before closing the notebook, which converts all the cells into static images/videos like it is the behaviour before this pr.

[behackl]

I've looked at this again today; it's been a while. :-)

While I like the general idea of supporting better image viewers within Jupyter notebooks, I am against "reinventing the wheel" here. I feel we should rather push towards compatibility with an image viewing library (like, e.g., Napari) instead of implementing it ourselves.

I'm happy to continue discussion about it here, but in the spirit of cleaning up, I'll close this PR for now. (Please re-open if you disagree!)