-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
[feature request] Use Enums for enumerated types #14642
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
Comments
+/-0 on this. On the one hand, I understand all your arguments for enums. On the other hand, the code gets more verbose:
And there is still the problem that you need to bring the constants into the namespace, ether via extra imports or by qualifying Someone should really write a completer that parses numpydoc and suggests string values defined therein as possible parameter values 😄. See also davidhalter/jedi#1355. |
This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help! |
Wouldn't it be possible to support both? It should be pretty simple to turn the Enum to the correct string (making internal changes minimal and/or postponable). Either way, dropping strings is not an option until mpl 4, if even then. Edit: in 3.11 |
Yes, that was my intended upgrade path. Some clever dickens has even made
VScode's python plugin does this if the string values are in the type annotations (e.g. |
My 2c (I know this is very old). VS Code auto-completes if the type is |
VS Code also autocompletes enum variants; I strongly suspect PyCharm does too. |
You would do This would also allow some doc rearrangement so that functions which use these enums (and in particular, functions which re-use the same enums) don't need such monolithic docs, as the enum itself can contain docs with details about the different variants.
|
We started trying to do that w/ a GSoD project a few years back - #18544, #18664 and the page is maybe clunky but a proof of concept https://matplotlib.org/stable/api/_enums_api.html#matplotlib._enums.JoinStyle |
@clbarnes perhaps I'm missing something. When you say "People can continue to use magic strings if they want (i.e. this is not a breaking API change)" - are you taking into account that both VS Code and PyCharm will show strings as being a warning/error? I would consider that quite undesirable, even if technically it doesn't count as a breaking API change because the code would still run. As you said, I think the suggestion to use enums back in 2019 was a good one, but with |
I agree that I therefore suggest to close this as not planned. As a side note: Unfortunately, neither of both approaches will help for the original example of |
Just like any deprecation, I'd argue. Red squigglies telling people to use proper programming patterns are a feature, not a bug. Literals get a chunk of the way to what I opened this for, and the remainder has its negatives as well as positives, so I'm happy for this to be closed if that's what the project decides. |
It would be nice if there were a mechanism to fix kwarg forwarding for doc strings/ type hints. |
I believe that's covered by PEP-692. |
ping @ksunden if that is something we may want to consider. |
RE: enums and type hints For those types that we have defined enums, I do accept the Enum type as well as the Literal strings for each option. These have a reusable name held in matplotlib/lib/matplotlib/typing.py Lines 54 to 56 in 939a332
This should hopefully provide IDE integrations that can understand the allowed values appropriately. There are some other common Literal groupings that are also represented in I'd be okay with saying If we really want to go down the route of "enums are not public and only an implementation detail", it may make sense to remove them from the I could see advocating with type checkers to always allow literal values where an Enum type is type hinted (allowing a "best of both worlds" where the type hint is just the enum, but editors know that the literals are allowed) but have no sense of how difficult that would be in practice. |
RE kwargs pass through I was fairly conservative with the first pass, and did not try and address this (common) pattern in type hints. It's not that I'm unwilling, just that it was a separable enough task that could incur changes with how runtime is written (though hopefully not how it behaves) There are a few PEPs/typing changes that potentially solve at least parts of the problem, but not sure any of them are as complete of a solution as I'd like (though may provide the building blocks for it). It is one of the less stable parts of typing, and many of the features are not supported by all currently supported python versions. Some have implementations in Various PEPs enter into the discussion: PEP-612
This example from #26367 shows what using PEP-692 may look like (for the internal deprecation machinery: matplotlib/lib/matplotlib/_api/deprecation.pyi Lines 15 to 33 in c686b2e
Here it is only actually used in the I have not experimented to see how the |
Matplotlib uses enumerated types, or something like them, in a number of places: for example, when setting locations ("upper", "lower" etc.). Historically, the pattern here has been to use strings. However, this is undesirable:
Instead, we could use an enum.Enum, which solves 1 and 2, and renders 3 moot.
By using an enum which also subclasses str, existing code would not break, and people could still pass in strings if they wanted to. No new code would be required in functions which take these strings; they'd just need to be defined once.
Alternatively, for e.g. locations, an
IntFlag
could be used so that "upper right" could beLocation.UPPER & Location.RIGHT
(this would probably require some internal changes). No more need to remember which way round the specifiers go, and whether it's"upper"
or"top"
: my IDE knows so I don't have to (and nor does anyone coming to the concept for the first time).One use case is for named colors. It would be trivial for IDEs to tell me what named colors are available, and check my spelling, but as it is, I'm googling the docs page every time I need them. I made a trivial package which does this: https://github.com/clbarnes/mpl_colors . The enum instances are also instances of an RGB namedtuple, which is also convenient for stacking them into an array, converting them into other spaces, comparing them to other color tuples, and so on.
The text was updated successfully, but these errors were encountered: