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

Enum cannot set default value using a dictionary #389

Closed
kitchoi opened this issue Jan 10, 2018 · 6 comments
Closed

Enum cannot set default value using a dictionary #389

kitchoi opened this issue Jan 10, 2018 · 6 comments
Milestone

Comments

@kitchoi
Copy link
Contributor

kitchoi commented Jan 10, 2018

Not a bug but suggestion for enhancement.

The following usage is currently not supported:

from traits.api import Dict, Enum, HasTraits, Int, Str

class EnumWithDictDemo(HasTraits):

    name_to_index = Dict(Str, Int)

    name = Enum(values="name_to_index")

demo = EnumWithDictDemo()
demo.name_to_index = {"A": 1, "B": 2}
print(demo.name)
Traceback (most recent call last):
  File "EnumEditor_Dict_demo.py", line 11, in <module>
    print(demo.name)
  File "/Users/kchoi/.edm/envs/traitsui/lib/python2.7/site-packages/traits/trait_types.py", line 2048, in _get
    value = values[0]
KeyError: 0

It would make sense to simply use value = next(iter(values)) for the default value?

@kitchoi kitchoi changed the title Enum can set default value using a dictionary or iterable in general Enum cannot set default value using a dictionary Jan 10, 2018
@mdickinson mdickinson added this to the 6.0.0 release milestone Jan 23, 2020
@mdickinson
Copy link
Member

Needs to be re-examined after #685 goes in, and before the 6.0 release.

@mdickinson
Copy link
Member

This now apparently works after #685 went in, though I regard the fact that it works as accidental, and it should not be assumed that it will carry on working in the future. The supported types for the values collection are list, tuple and EnumMeta.

@kitchoi
Copy link
Contributor Author

kitchoi commented Jan 24, 2020

If users should not assume this to continue working and yet it works, it might be better if it did not work at all...
What about testing reversed(values) and raise if the values cannot be reversed (hence reopening the issue for future reconsideration)?

@kitchoi
Copy link
Contributor Author

kitchoi commented Jan 24, 2020

testing reversed(values)

Actually, list, tuple and EnumMeta satisfy the condition of isinstance(values, abc.Reversible) and isinstance(values, Collection).

@mdickinson
Copy link
Member

it might be better if it did not work at all...

Agreed: I'd rather have it working, tested and documented, or not working at all. But adding new validation where there was none before is going to be a can of worms; we'd probably need at least a deprecation warning. This is something we could think about for 6.1.0

@kitchoi
Copy link
Contributor Author

kitchoi commented Mar 30, 2020

For future reference, #968 is going to make Enum support dictionary (and friends).

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

No branches or pull requests

2 participants