Repository navigation
[feature request] Use Enums for enumerated types #14642
Description
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:
- Values are not discoverable without reading the docs
- Values do not autocomplete and cannot be checked for correctness by most IDEs
- Remembering the canonical dashes, underscores and capitals is unnecessarily complicated
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 be Location.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.
Activity
timhoffm commented on Jun 29, 2019
+/-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 plt.Location.LEFT or mpl.Location.LEFT, which would make the usage even more verbose.
Someone should really write a completer that parses numpydoc and suggests string values defined therein as possible parameter values 😄. See also davidhalter/jedi#1355.
github-actions commented on Jun 21, 2023
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!
oscargus commented on Jun 21, 2023
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 StrEnum is introduced which will make this really straightforward. https://docs.python.org/3/library/enum.html#enum.StrEnum but for now something like mode = str(mode) with an Enum with an appropriately overridden __str__-method should do the trick.
clbarnes commented on Jun 21, 2023
Yes, that was my intended upgrade path. Some clever dickens has even made backports.strenum which, in a few days time, will cover all non-EOL python versions on recent patches ;) .
VScode's python plugin does this if the string values are in the type annotations (e.g. Literal["left", "right"]).
davidgilbertson commented on Jul 24, 2023
My 2c (I know this is very old). VS Code auto-completes if the type is Literal, PyCharm doesn't, but will warn if a value is wrong, and the options are visible with ctrl+Q for 'quick info'. Not sure about other IDEs, but the point is defining the type as Literal gets you a long way toward the benefits of enums. If you were to implement enums, it would be worth checking that it's not actually a step backward for users who want to type the string in the major IDEs.
clbarnes commented on Jul 24, 2023
VS Code also autocompletes enum variants; I strongly suspect PyCharm does too.
davidgilbertson commented on Jul 24, 2023
I've never done a string enum before, if the below is right then nope, I'm afraid neither VS Code nor PyCharm realise that "left" is valid here:
Am I doing it wrong?
Meanwhile, with Literal, you get hints in PyCharm, (full auto-complete in VS Code)
And a warning if you enter something invalid:
clbarnes commented on Jul 25, 2023
You would do take_direction(Direction.<TAB>) to make use of autocompletion, discoverability, and improved docs. People can continue to use magic strings if they want (i.e. this is not a breaking API change), but my general feeling (which shouldn't be too controversial) is that enums should be represented by enums.
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.
Literal definitely eats into the utility of this change somewhat. In my defence, this issue was created only a couple of months after PEP586 was written; months before it was implemented in 3.8 and so years before python versions without Literal went EOL.
story645 commented on Jul 25, 2023
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
davidgilbertson commented on Jul 25, 2023
@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 Literal available as an option, coupled with the fact that enums would introduce errors/warnings in user code, makes it not such a good option now.
timhoffm commented on Jul 25, 2023
I agree that Literal is a good solution today. Enums are quite cumbersome because you have to import them (or their containing namespaces) explicitly. Also, the user code gets longer and less readable. In #18433, #18664 we explicitly decided to keep these type classes an implementation detail and continue to use string literals for the public API.
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 legend(..., loc=...) (and many similar cases) because these parameters are forwarded via **kwargs and thus not transparent to type checkers and completers.
clbarnes commented on Jul 26, 2023
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.
jklymak commented on Jul 26, 2023
It would be nice if there were a mechanism to fix kwarg forwarding for doc strings/ type hints.
clbarnes commented on Jul 26, 2023
I believe that's covered by PEP-692.
jklymak commented on Jul 26, 2023
ping @ksunden if that is something we may want to consider.
ksunden commented on Jul 26, 2023
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 typing.py which is used as the type hint for things that expect those enums:
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 typing.py (FillStyleType, for one, from above) that have reusable names as well. Though there are many places where I just copied a e.g. Literal["left", "center", "right"] around. (And more that feel like a Literal/enum would be useful but actually have more options than are reasonable to enumerate, e.g. hatch pattern combinations)
I'd be okay with saying Literal is the path forward for many of these types in current python, given that it has editor integration, avoids code changes to user code (and imports).
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 typing.py defined types.
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.
ksunden commented on Jul 26, 2023
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 typing_extensions, which do backport, but I had been avoiding using that to fully avoid adding it as a (direct) dependency. Now, I think that it is already a transitive dependency, and is at least safe to use as a type-checking dependency (I think even mypy depends on it), even if we avoid it as a runtime dependency, so probably not the worst dependency to take on, but trying to avoid it if possible.
Various PEPs enter into the discussion:
PEP-612
PEP-646
PEP-692 (mentioned above)
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 pyi files, and using total=False in the class construction helps with the required/not required behavior.
I have not experimented to see how the stubtest ci manages if these get out of sync with the implementation, etc.