-
Notifications
You must be signed in to change notification settings - Fork 947
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
Allow Mappings to be given as the options in a selection widget again. #3557
Conversation
We deprecated this long ago when dicts were not ordered (jupyter-widgets#2130, jupyter-widgets#1958), then removed it in ipywidgets 8.0. However, now dicts are ordered by default, so perhaps we can put it back in and keep people's code from breaking.
The expected tests fail. If we decide to do this PR, I can fix the tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a general consensus in the dev meeting today that it was probably best to add back the support now that dicts are ordered by default, but to not put any use/mention of this pattern in the docs. @maartenbreddels also argued for not mentioning it in the docstring either, but instead just leave a comment in the code that this support is here for backwards compatibility reasons.
Tests will then need fixing. |
This essentially does a partial reversion of 9db179b and 28877f9 (jupyter-widgets#2679)
I believe the warning was shown as suggested by the second screenshot in #2679 (comment)? |
I'll leave off the merge for now to leave some time for others to comment, but looks ok to me! |
Only if you reset the DeprecationWarning filter to always show deprecation warnings (or ran Python with the appropriate switches to always show deprecation warnings, etc.), as shown in that screenshot. I imagine a vanishingly small fraction of our users did this? I guess what I realized last night was that Python has had a pretty bad story about surfacing deprecation warnings to users for a lot of the 3.x series, and as mentioned in the threads linked in #3569, it is tricky to get the appropriate stacklevel so that even the much better automatic surfacing of warnings in Python 3.7+ works. |
I stared at this for a while before I realized we had check_widget and check_widgets. This rename makes the distinction between these functions more clear.
Oh right, then yes probably not many users were able to see it. In that case yes it makes sense to revert it since most users probably didn't get the notice 👍 |
…hat we reverted the change. Follows up on jupyter-widgets#3557 to correct the docs
We deprecated this long ago when dicts were not ordered (#2130, #1958, #2679), then removed it in ipywidgets 8.0. However, now dicts are ordered by default, so perhaps we can put it back in and keep people's code from breaking.
I want to place a high value on easy ways to support backwards compatibility, as many people would rather just not change their code if they can help it.
However, if we are going to stick with this decision in 8.0, #3556 makes the error more helpful. #3556 and this PR are mutually exclusive - at most one should be merged.