Augment base enumeration class to behave well in string contexts#192
Augment base enumeration class to behave well in string contexts#192
Conversation
|
Can I get some context how this is being used? I'm curious why we need to be able to handle strings with mixed lower/uppercase. Where are these strings coming from? |
If you look at routes supported by citrine-python, you'll see several ostensibly enumerated field entries that are only being represented as strings. You'll also note the casing choice on those enumerations is actually end-point specific. This removes sensitivity to a user knowing the casing details of particular routes, through giving access to a Enum that still behaves cleanly in a string context, case-insensitive object deserialization, and easing library interactions. |
anoto-moniz
left a comment
anoto-moniz
left a comment
There was a problem hiding this comment.
Seems reasonable to me. I presume you have some place in mind you'd like to use this?
Every instance of citrine-python's use of BaseEnumeration. Plus they're just more pleasant. They've been in gemd-ingest for a long time. |
|
Can you clarify the asserts? I'm confused by line 61 especially. |
The synonym mechanism allows you to have multiple inputs that map to the same enumerated value, but equality doesn't work because the string form is always the first entry: |
| self._template = None | ||
| self._origin = None | ||
| self._file_links = None, | ||
| self._file_links = None |
There was a problem hiding this comment.
Collateral discovery. Just a cosmetic issue b/c the attribute is initialized 4 lines later.
|
|
||
| @property | ||
| def origin(self) -> str: | ||
| def origin(self) -> Origin: |
There was a problem hiding this comment.
Origin is a str, so this is non-breaking and more intuitive.
|
|
||
|
|
||
| def validate_list(obj: Union[Iterable[T], T], | ||
| def validate_list(obj: Optional[Union[Iterable[T], T]], |
There was a problem hiding this comment.
Collateral discovery. It accepts a None, initializing to empty list, so why is that not documented?
| assert copy_condition.notes == Origin.UNKNOWN.value | ||
| assert not isinstance(copy_condition.notes, Origin) |
There was a problem hiding this comment.
get_value is deprecated, plus this is a better test anyway.
StevenKauwe
left a comment
StevenKauwe
left a comment
There was a problem hiding this comment.
LGTM! The added doc strings are very helpful ❤️.
3 participants
This PR modifies the Base Enumeration class to behave better in string contexts, as well as support synonyms and better string matching.
This does not change any behavior that was previously under testing nor change any previously defined interfaces.