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

Pin ipywidget version to 7 #1033

Merged
merged 4 commits into from
Sep 12, 2022
Merged

Pin ipywidget version to 7 #1033

merged 4 commits into from
Sep 12, 2022

Conversation

pmrv
Copy link
Contributor

@pmrv pmrv commented Aug 22, 2022

Following the discussion in #1032 and here, it seems that nglview is only compatible with version 7 of ipywidgets. So I went ahead and pinned this in the environment.yml and setup.py. Feel free to close this again if I've misjudged the situation.

setup.py specifies >=7 is required, but version 8 introduced breaking changes
setup.py specifies >=7 is required, but version 8 introduced breaking changes
@hadim
Copy link
Contributor

hadim commented Aug 22, 2022

I also got the same bug recently (AttributeError: 'super' object has no attribute '_ipython_display_'). Would that be possible to instead make nglview compatible with ipywidget=8? It should be possible to make it compatible with both versions, actually.

(ping @maclandrol for visibility)

@pmrv
Copy link
Contributor Author

pmrv commented Aug 22, 2022

I definitely don't have the insight into the code for bigger changes, but I'd like to have the conda package working again without third party packages being required to work around the situation, so I thought this might be a good start.

@hainm
Copy link
Collaborator

hainm commented Aug 23, 2022

Thanks @pmrv: it seems it would take much time for 7 to 8 migration: https://ipywidgets.readthedocs.io/en/latest/migration_guides.html#migrating-from-7-x-to-8-0

@hainm
Copy link
Collaborator

hainm commented Aug 23, 2022

Would that be possible to instead make nglview compatible with ipywidget=8? It should be possible to make it compatible with both versions, actually.

@hadim yeah, it's always possible but it's not as simple as one line fix. Please see the link from my previous post. Cheers.

@pmrv
Copy link
Contributor Author

pmrv commented Aug 23, 2022

I'm a bit confused about the failing test. At the top the workflow prints 'nglview/tests/test_cli.py::test_cli NOTE: make sure to open tmpnb_ngl.ipynb in your local machine'. Are there additional steps I need to do in a local check out for the CI to work?

@hainm
Copy link
Collaborator

hainm commented Aug 23, 2022

I'm a bit confused about the failing test. At the top the workflow prints 'nglview/tests/test_cli.py::test_cli NOTE: make sure to open tmpnb_ngl.ipynb in your local machine'. Are there additional steps I need to do in a local check out for the CI to work?

hi @pmrv

Those are the failures:

1

                b = HBox([self, self._gui])
                def on(b):
                    self.handle_resize()
>               b.on_displayed(on)
E               AttributeError: 'HBox' object has no attribute 'on_displayed'

2

>   from notebook.notebookapp import NotebookApp, random_ports
E   ModuleNotFoundError: No module named 'notebook'

3

>       for k, v in views[0].widgets.items():
E       AttributeError: 'NGLWidget' object has no attribute 'widgets'

4

        Widget._comm_default = lambda self: DummyComm()
>       _widget_attrs['_ipython_display_'] = Widget._ipython_display_
E       AttributeError: type object 'Widget' has no attribute '_ipython_display_'

@hainm
Copy link
Collaborator

hainm commented Aug 23, 2022

@pmrv so it seems to me that this test still installed ipywidgets v8.

setup.py Outdated Show resolved Hide resolved
@hainm
Copy link
Collaborator

hainm commented Aug 25, 2022

@pmrv please update here to pass the test too: https://github.com/nglviewer/nglview/blob/master/.github/nglview-gha.yml#L13

@niklassiemer
Copy link

@pmrv ping

@pmrv
Copy link
Contributor Author

pmrv commented Sep 12, 2022

I've added the pin in the indicated file and also in the recipes in devtools directories. I've also grepped with ag 'ipywidgets.{0,5}(>=|=|==|<|$)' and didn't find further hidden version specs.

Sorry for the delay, but I'm currently hopping between conferences and will not be able to respond in a timely manner.

@hainm hainm merged commit c3fe543 into nglviewer:master Sep 12, 2022
@hainm
Copy link
Collaborator

hainm commented Sep 12, 2022

Thanks @pmrv for you work. Cheers.

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.

None yet

4 participants