Skip to content

Fix test output to pass with ipywidgets 8.0.5 #35336

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

Jump to bottom
Merged
merged 1 commit into from
Apr 6, 2023
Merged

Fix test output to pass with ipywidgets 8.0.5 #35336

merged 1 commit into from
Apr 6, 2023

Conversation

antonio-rojas
Copy link
Contributor

With ipywidgets 8.0.5, the widget text output gets a new continuous_update parameter that makes a test fail.

There are some other test failures in sage/combinat/cluster_algebra_quiver due to an upstream bug, which is fixed in jupyter-widgets/ipywidgets@8123715

@github-actions
Copy link

Documentation preview for this PR is ready! 🎉
Built with commit: 453f3ba

@tornaria
Copy link
Contributor

LGTM, but I don't have ipywidgets 8.0.5 yet, so I can't easily test.

@kiwifb
Copy link
Member

kiwifb commented Mar 23, 2023 •
edited
Loading

I am only on 8.0.4 which doesn't exhibit that particular problem. But I have deprecation warnings polluting my doctest at the same spot. And it is about on_submit being deprecated and that you should set the .on_continous_update to false and use mywidget.observe. It does echo your description about continuous_update. I think this is another of those cases where we should be a bit more thorough than just fixing the doctest.

@antonio-rojas
Copy link
Contributor Author

I am only on 8.0.4 which doesn't exhibit that particular problem. But I have deprecation warnings polluting my doctest at the same spot. And it is about on_submit being deprecated and that you should set the .on_continous_update to false and use mywidget.observe. It does echo your description about continuous_update. I think this is another of those cases where we should be a bit more thorough than just fixing the doctest.

That's a bug in ipywidgets itself that has been fixed in 8.0.5. See jupyter-widgets/ipywidgets#3669

@kiwifb
Copy link
Member

kiwifb commented Mar 23, 2023

Then it is all good to me. Can't wait to update to get rid of that deprecation warning. I am swimming in them in sage-on-gentoo.

@tornaria
Copy link
Contributor

Unfortunately (or fortunately) all the jupyter-* stuff in void is a bit outdated (most packages are orphaned) and since I almost don't use notebooks, it hasn't been my priority to update (also: jupyter is like a zillion interrelated packages that have to be updated with some correlations, and there is no clear documentation on what works when and how, so it's not so easy to do it in small steps).

@vbraun vbraun merged commit 7ab3e3a into sagemath:develop Apr 6, 2023
@collares collares mentioned this pull request Apr 13, 2023
Merged
5 tasks
vbraun pushed a commit that referenced this pull request Apr 23, 2023
41c256a 
gh-35499: Fix test output for ipywidgets 8.0.5, part deux
    
### 📚 Description

Apply the change introduced in #35336 to the corresponding `.rst` file
as well.

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies
    
URL: #35499
Reported by: Mauricio Collares
Reviewer(s): François Bissey
@antonio-rojas antonio-rojas deleted the ipywidgets-8.0.5 branch July 30, 2023 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tornaria tornaria Awaiting requested review from tornaria

1 more reviewer

@kiwifb kiwifb kiwifb approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
c: packages: standard t: tests
Projects
None yet
Milestone
sage-10.0
Development

Successfully merging this pull request may close these issues.
5 participants
@antonio-rojas @tornaria @kiwifb @vbraun @mkoeppe