Let UseEnum select a member by value, not only by name or number - #979
Open
sap1110 wants to merge 1 commit into
Open
Let UseEnum select a member by value, not only by name or number#979sap1110 wants to merge 1 commit into
sap1110 wants to merge 1 commit into
Conversation
UseEnum accepts a member's name, its scoped name, and (for int-valued enums)
its number, but it has no lookup by value. A str-based enum whose member names
differ from their values therefore cannot be set by value at all:
class Color(str, enum.Enum):
RED = "red"
GREEN = "green"
class MyEntity(HasTraits):
color = UseEnum(Color)
MyEntity().color = "GREEN" # worked, matched the member name
MyEntity().color = "green" # TraitError, though it is Color.GREEN's value
select_by_value mirrors the existing select_by_number for non-numeric values,
and the string branch of validate falls back to it. Name lookup is still tried
first, so any string that resolved before resolves to the same member now; the
fallback only runs where a TraitError was raised before. A test covers an enum
whose member name shadows another member's value to pin that ordering.
Codecov Report✅ All modified and coverable lines are covered by tests.
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #827
UseEnumresolves a member from its name, its scoped name, or (for int-valued enums) its number. There is no lookup by value, so astr-based enum whose member names differ from their values cannot be set by value:The docstring example hides this because its member names are lowercase and equal to nothing else, so
"green"happens to match a name. Give the members conventional uppercase names and the string form stops working entirely.select_by_valueis the non-numeric counterpart of the existingselect_by_number, and the string branch ofvalidatenow falls back to it.The change is additive. Name lookup is still attempted first, so every string that resolved before resolves to the same member; the new lookup only runs where a
TraitErrorwas previously raised. There is a test using an enum whose member name shadows a different member's value, so the precedence is pinned rather than incidental.One thing worth a second opinion:
info()still lists only member names, so the error text for a genuinely bad value readsexpected any of ['RED', 'BLUE', 'GREEN']even though values are now accepted too. Widening it would change message text that downstream suites may match on, so I left it alone. Happy to include it if you'd rather they agree.🤖🍆 Prepared with the help of an AI coding agent, marked as the contributing guide asks agents to do. Reviewed and tested by me.