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

Drop support for mutable types as dropdown options #1958

Closed
fterbo opened this issue Feb 5, 2018 · 3 comments · Fixed by #2679 or #2130
Closed

Drop support for mutable types as dropdown options #1958

fterbo opened this issue Feb 5, 2018 · 3 comments · Fixed by #2679 or #2130
Labels
good first issue resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Milestone

Comments

@fterbo
Copy link

fterbo commented Feb 5, 2018

I know this has been reviewed before, but I humbly request it be reviewed again.. :-)

I don't see any upside to supporting dictionaries as an options list (you can just use widget.options = dict.items() if you want the same behaviour, which more clearly separates ownership semantics), and it is a source of many pitfalls. I suspect that many people use dictionaries simply because it's more prominently featured in the docs than the list-of-tuples approach.

I realize this is a backwards-incompatible change, but I think moving forward it would reduce community support burden (and frustrated user time). Certainly this could be done in a phased manner with DeprecationWarning (making sure users can actually see it, of course).

@jasongrout
Copy link
Member

I definitely agree.

@jasongrout jasongrout added this to the Major release milestone Feb 6, 2018
@jasongrout
Copy link
Member

jasongrout commented Feb 6, 2018

We can start by triggering a deprecation message when we see Mappings (e.g., dictionaries) here:

# Check if x is a mapping of labels to values
if isinstance(x, Mapping):
return tuple((unicode_type(k), v) for k, v in x.items())

(and change the docs and examples to not use dictionaries)

Later for 8.0 we can remove that case entirely.

@jasongrout
Copy link
Member

Deprecation added in 7.4. We still need to actually remove support in 8.0 (i.e., a major release)

@lock lock bot added the resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label May 20, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
2 participants