Add StrEnum conversion support #177
Conversation
|
|
temporalio/converter.py
Outdated
| @@ -874,6 +874,14 @@ def value_to_type(hint: Type, value: Any) -> Any: | |||
| ) | |||
| return hint(value) | |||
|
|
|||
| # StrEnum | |||
| if inspect.isclass(hint) and issubclass(hint, StrEnum): | |||
There was a problem hiding this comment.
@cretz,
Oops, I forgot StrEnum is not in the older Python version.
We can change it to
if inspect.isclass(hint) and issubclass(hint, str) and isinstance(hint, Enum):ref)
There was a problem hiding this comment.
It may be better if we did a:
if sys.version_info >= (3, 11):
from enum import StrEnumAnd put that same conditional around this conversion code and around the test
There was a problem hiding this comment.
Yup that would work too. Let me update my commit
There was a problem hiding this comment.
Added another commit accordingly. Please review the new commit 🙏
Co-authored-by: Chad Retz <[email protected]>
tests/test_converter.py
Outdated
| @@ -46,6 +46,10 @@ class SerializableEnum(IntEnum): | |||
| FOO = 1 | |||
|
|
|||
|
|
|||
| class SerializableStrEnum(StrEnum): | |||
There was a problem hiding this comment.
We run tests all the way back to Python 3.7, so you'll need to make this class (and its required import and its check in the test) conditional on 3.11 too
There was a problem hiding this comment.
Updated the test and confirmed both the 3.7 and 3.11 Python versions passes the test on my local machine. I should've checked it in the first place ^^; Please check the change 🙏
What was changed
Add support for string-based Enum
Why?
String-based enum is apparently widely used, along with int-based enums. Python-sdk already has a support int-based enum, and it makes sense also to support another most popular enum, string.
Checklist
Closes [Feature Request] Support string-based enum #176
How was this tested:
Tested with updated unit test
Any docs updates needed?
Readme was slightly updated