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.

Sign up for GitHub

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

Add potential fix for increasing map size bug #3699

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add potential fix for increasing map size bug #3699

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7143969
Add potential fix for increasing map size bug
filipre Feb 9, 2023
b8a89ab
Make arguments explicit and add documentation
filipre Feb 9, 2023
10aae64
Add mode for manager state
filipre Feb 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions python/ipywidgets/ipywidgets/embed.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@

import json
import re
from .widgets import Widget, DOMWidget, widget as widget_module
from .widgets.widget_link import Link
from .widgets.docutils import doc_subst

from ._version import __html_manager_version__
from .widgets import DOMWidget, Widget
from .widgets import widget as widget_module
from .widgets.docutils import doc_subst
from .widgets.widget_link import Link

snippet_template = """
{load}
Expand Down Expand Up @@ -299,7 +301,21 @@ def embed_minimal_html(fp, views, title='IPyWidget export', template=None, **kwa
will be replaced by all the widgets.
{embed_kwargs}
"""
snippet = embed_snippet(views, **kwargs)

if kwargs.get("state") is None:
state = dependency_state(views, drop_defaults=kwargs.get("drop_defaults"))
else:
state = kwargs["state"]

snippet = embed_snippet(
views,
drop_defaults=kwargs.get("drop_defaults", True),
state=state,
indent=kwargs.get("indent", 2),
embed_url=kwargs.get("embed_url", None),
requirejs=kwargs.get("requirejs", True),
cors=kwargs.get("cors", True)
)
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be good to explicitly add the arguments (just copy-paste them from embed_snippet).
Would also be good to update the docstring, which is not incorrect (the behaviour of state=None is different).
Maybe this should be an opt-in for backward compatibility. (state_tree_shake=False maybe?)

Copy link
Author

Choose a reason for hiding this comment

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

If the correct behaviour is dependency_state and not get_manager_state, then I think the backwards compatibility relies on a bug 🤔 What do you think about passing a string instead, e.g. state="all" to include everything instead of having an argument toggle that switches between dependencies and full state (and only if state is None)

Copy link
Author

Choose a reason for hiding this comment

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

I updated the documentation and also used a string to toggle between the modes:

state is None or equals "dependent": function will use dependency_state:

Current: 0.html: 0.0067501068115234375 MB
Current: 1.html: 0.0067501068115234375 MB
Current: 2.html: 0.0067501068115234375 MB
Current: 3.html: 0.0067501068115234375 MB
Current: 4.html: 0.0067501068115234375 MB
...

state equals "complete": function will use get_manager_state:

Current: 0.html: 0.006749153137207031 MB
Current: 1.html: 0.012719154357910156 MB
Current: 2.html: 0.01868915557861328 MB
Current: 3.html: 0.024659156799316406 MB
Current: 4.html: 0.03062915802001953 MB
...

state is a dictionary: function will use passed state direclty

Current: 0.html: 0.0007772445678710938 MB
Current: 1.html: 0.0007772445678710938 MB
Current: 2.html: 0.0007772445678710938 MB
Current: 3.html: 0.0007772445678710938 MB
Current: 4.html: 0.0007772445678710938 MB
...

Could you take another look on the code?


values = {
'title': title,
Expand Down