-
Notifications
You must be signed in to change notification settings - Fork 174
enumToStr #325
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
base: master
Are you sure you want to change the base?
Conversation
…for direct conversion to string - mostly useful for verbose to-string implementations
|
@GreyCat Thanks. I haven't looked at the implementation, but I agree that we need this, otherwise |
|
Thanks, added to the summary! The implementation is relatively simple, feel free to take a look when you'll be available. |
| override def enumToInt(v: Ast.expr, et: EnumType): String = | ||
| s"int(${translate(v)})" | ||
| override def enumToStr(v: Ast.expr, et: EnumType): String = | ||
| s"${translate(v)}.name" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will only work for known/defined enum values, but fail for unknown enum values, which are represented as int in Python and thus won't have the .name attribute.
Since we use IntEnum as the base class for the enums, it isn't super simple to implement this in a way that works for both known and unknown enum values. str(enum_val) will always give you the stringified integer value, since IntEnum tries to be compatible with int. This is nicely explained in https://docs.python.org/3/library/enum.html#notes:
These three enum types are designed to be drop-in replacements for existing integer- and string-based values; as such, they have extra limitations:
__str__uses the value and not the name of the enum member__format__, because it uses__str__, will also use the value of the enum member instead of its nameIf you do not need/want those limitations, you can either create your own base class by mixing in the
intorstrtype yourself:>>> from enum import Enum >>> class MyIntEnum(int, Enum): ... passor you can reassign the appropriate
str(), etc., in your enum:>>> from enum import Enum, IntEnum >>> class MyIntEnum(IntEnum): ... __str__ = Enum.__str__
Actually, we'll want to override __str__ with our own implementation so that it returns just the member name without the enum name as a prefix (like enum.Enum.__str__ does), as shown in the example for https://docs.python.org/3/library/enum.html#enum.Enum.__str__:
def __str__(self):
return self.nameThen the compiler can translate the enum_val.to_s simply as str(enum_val) and it will return the enum member name for known values and stringified integer for unknown values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's an interesting design question.
Up until now, we've respected per-language design, which pretty much boils down to some languages retaining existing undeclared integer value (e.g. C/C++, JavaScript, Python), and some (e.g. Java or Ruby) having no default way to keep it stored:
- Java will pretty much have
nullon invalid enums — and it's rather hard to do anything about that - Ruby will have
nilin its current implementation we have — but, to be fair, we can influence that.
This pretty much means that if we'll try to validate it with tests, we won't get consistent results.
I can think of 3 possible choices here for handling invalid enum values:
- Don't do anything — leave it different, some will blow up with NPE, don't care
- Lowest common denominator — consistently return a fixed string like
<UNKNOWN>or<INVALID>for all languages. - Have similar but slightly different values:
- For languages who keep the value — return something like
unknown(123) - For languages who can't keep — return something like
unknown()
- For languages who keep the value — return something like
If we'll do (2) or (3) — we can make the test: (2) with equality, (3) with something like "starts with unknown(".
Any preferences?
Adds
enumToStrimplementation, allowing.to_son enum for direct conversion to string, implementing request in kaitai-io/kaitai_struct#979.This is mostly useful for verbose to-string implementations, to eventually replace
-webide-representationwhich allows enum-to-string conversions.The approach taken is the simplest and most bare bones:
pet.to_swherepet == animal::catis expected to return"cat"in original .ksy spelling (lower underscore case).Implementations so far available for:
Other languages so far have
???as implementation, thus it won't break the compilation overall but will fail in runtime with Scala.NotImplementedError if attempted to be used.Companion test PR: kaitai-io/kaitai_struct_tests#137