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

Update to ipywidgets 8 #282

Merged
merged 44 commits into from
Sep 30, 2022
Merged

Update to ipywidgets 8 #282

merged 44 commits into from
Sep 30, 2022

Conversation

ibdafna
Copy link
Member

@ibdafna ibdafna commented Dec 7, 2021

Signed-off-by: Itay Dafna i.b.dafna@gmail.com

@ibdafna ibdafna marked this pull request as draft December 7, 2021 03:50
@ibdafna
Copy link
Member Author

ibdafna commented Dec 7, 2021

@martinRenou I seem to be getting an type issue in the ipywidgets library when compiling ipydatagrid, really odd:

/node_modules/@jupyter-widgets/base/lib/widget".WidgetView' has no exported member named 'InitializeParameters'. Did you mean 'IInitializeParameters'?

Wondering what's going on. Can you see anything off with my updates?

@martinRenou
Copy link
Member

@ibdafna You should probably rename it to IInitializeParameters for it to compile. This structure has been renamed in jupyter-widgets/ipywidgets@5221e4d#diff-efb19099381ae8911dd7f69b015a0138d08da7164512c1ee112aa75100bc9be2R694

@ibdafna
Copy link
Member Author

ibdafna commented Dec 7, 2021

@ibdafna You should probably rename it to IInitializeParameters for it to compile. This structure has been renamed in jupyter-widgets/ipywidgets@5221e4d#diff-efb19099381ae8911dd7f69b015a0138d08da7164512c1ee112aa75100bc9be2R694

Yeah, I'm not sure why this is happening because it's coming from @jupyter-widgets/base package on the latest beta version. Perhaps the rename is not reflected in the release?

EDIT: quite silly of me - I didn't rename this in the extension file.

package.json Outdated Show resolved Hide resolved
@ibdafna
Copy link
Member Author

ibdafna commented Apr 20, 2022

@martinRenou I took another stab at fixing the jest spyOn() issue. I get a more meaningful error message now and it seems to be related to our mock comm of the mock widget manager.

 FAIL  tests/js/datagrid.test.ts
  ● Test suite failed to run

    tests/js/datagrid.test.ts:37:29 - error TS2769: No overload matches this call.
      Overload 1 of 4, '(object: {}, method: never): SpyInstance<never, never>', gave the following error.
        Argument of type 'IClassicComm | undefined' is not assignable to parameter of type '{}'.
          Type 'undefined' is not assignable to type '{}'.
      Overload 2 of 4, '(object: {}, method: never): SpyInstance<never, never>', gave the following error.
        Argument of type 'IClassicComm | undefined' is not assignable to parameter of type '{}'.
          Type 'undefined' is not assignable to type '{}'.

    37     const mock = jest.spyOn(grid.model.comm, 'send');
                                   ~~~~~~~~~~~~~~~

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        3.469 s```

https://github.com/bloomberg/ipydatagrid/blob/2540fcf8edc946c00c218537114830dc0ad16bf4/tests/js/testUtils.ts#L155-L218

@ibdafna ibdafna force-pushed the ipywidgets_8 branch 2 times, most recently from 30e27ff to 0c2e839 Compare August 24, 2022 21:14
@ibdafna ibdafna marked this pull request as ready for review August 24, 2022 21:15
setup.cfg Outdated
@@ -28,7 +28,7 @@ keywords = Jupyter, Widgets, IPython
packages = find:
install_requires =
bqplot>=0.11.6
ipywidgets>=7.6.0,<8
ipywidgets>=7.6.0
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ipywidgets>=7.6.0
ipywidgets>=7.6.0,<9

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you!

@ibdafna
Copy link
Member Author

ibdafna commented Aug 25, 2022

galata update references

@ibdafna
Copy link
Member Author

ibdafna commented Aug 25, 2022

update galata references

1 similar comment
@ibdafna
Copy link
Member Author

ibdafna commented Aug 25, 2022

update galata references

@ibdafna ibdafna closed this Aug 25, 2022
@ibdafna ibdafna reopened this Aug 25, 2022
@ibdafna
Copy link
Member Author

ibdafna commented Aug 25, 2022

@martinRenou It seems like the Galata bot has updated the reference images with a images from a failed render attempt - they all show "error displaying widget model". The reference images are captures using ipywidgets 7, and testing locally with ipywidgetx 7.x works fine. Another thing to think about is that there we only capture reference images from one source - either the 7.x or 8.x environment. Should we think about extending the logic so we have separate images for both versions? Is it even reasonable to expect images to render (however slightly) differently between the two major versions?

@ibdafna
Copy link
Member Author

ibdafna commented Aug 25, 2022

update galata references

@martinRenou
Copy link
Member

they all show "error displaying widget model". The reference images are captures using ipywidgets 7

Are you able to capture the JS outputs for those tests? Jason shared some nice debugging tricks with Galata here jupyter-widgets/ipyleaflet#968 (comment)

Is it even reasonable to expect images to render (however slightly) differently between the two major versions?

I guess it's reasonable.
I've seen a slight shift to the right for some outputs in ipympl and bqplot, but I bet it's due to some change in Galata or JupyterLab, not ipywidgets.

@ibdafna
Copy link
Member Author

ibdafna commented Sep 11, 2022

@martinRenou the visual regression tests are genuinely failing on ipywidgets 7.x. We didn't think about the JupyterLuminoPanelWidget imports in ipywidgets 7.x. I added a function with some ts-ignores, but it's still failing. Need to investigate further

@ibdafna
Copy link
Member Author

ibdafna commented Sep 28, 2022

update galata references

@ibdafna ibdafna closed this Sep 28, 2022
@ibdafna ibdafna reopened this Sep 28, 2022
@ibdafna
Copy link
Member Author

ibdafna commented Sep 29, 2022

update galata references

1 similar comment
@ibdafna
Copy link
Member Author

ibdafna commented Sep 29, 2022

update galata references

ibdafna and others added 23 commits September 29, 2022 14:26
…sion

Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Bumps [shell-quote](https://github.com/substack/node-shell-quote) from 1.7.2 to 1.7.3.
- [Release notes](https://github.com/substack/node-shell-quote/releases)
- [Changelog](https://github.com/substack/node-shell-quote/blob/master/CHANGELOG.md)
- [Commits](https://github.com/substack/node-shell-quote/compare/v1.7.2...1.7.3)

---
updated-dependencies:
- dependency-name: shell-quote
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <idafna@seas.upenn.edu>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
…sion

Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>
@ibdafna ibdafna merged commit 2ef8706 into jupyter-widgets:main Sep 30, 2022
@ibdafna
Copy link
Member Author

ibdafna commented Sep 30, 2022

🎉🎊🥳

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.

4 participants