PEP 649: Change the defined format values to an IntEnum#3127
PEP 649: Change the defined format values to an IntEnum#3127larryhastings wants to merge 1 commit intopython:mainfrom
Conversation
larryhastings
changed the title
AA-Turner
left a comment
AA-Turner
left a comment
There was a problem hiding this comment.
One syntax error to fix, two other questions.
A
| in a new ``AnnotationFormat`` enum, which is an ``enum.IntEnum``. | ||
| The formats defined by this PEP are: | ||
|
|
||
| * ``inspect.VALUE = 1`` |
There was a problem hiding this comment.
Does this list also need updating?
There was a problem hiding this comment.
I'm not sure what you have in mind. The enum values are consistent between the two definitions, they're not out-of-date.
I'm gonna consider this "done", as in, there was no problem. If you still have feedback on this, please add it to the other PR, #3138. Thanks!
| FORMAT_MIN = AnnotationFormat.VALUE | ||
| FORMAT_MAX = AnnotationFormat.SOURCE |
There was a problem hiding this comment.
If now an enum, do these still make sense, as one could just iterate through the enum?
There was a problem hiding this comment.
I also don't see a strong use case for these, so maybe we should just drop them. We can always re-add them in a later version if a good use case comes up.
There was a problem hiding this comment.
Removed. Iterating through the enum is an even better solution.
There was a problem hiding this comment.
(work is in other PR)
| ``inspect`` module:: | ||
|
|
||
| ``` | ||
| class AnnotationFormat(enum.IntEnum): | ||
| VALUE = 1 | ||
| FORWARDREF = 2 | ||
| SOURCE = 3 | ||
| ``` |
There was a problem hiding this comment.
Syntax fix:
| ``inspect`` module:: | |
| ``` | |
| class AnnotationFormat(enum.IntEnum): | |
| VALUE = 1 | |
| FORWARDREF = 2 | |
| SOURCE = 3 | |
| ``` | |
| ``inspect`` module: | |
| .. code:: python | |
| class AnnotationFormat(enum.IntEnum): | |
| VALUE = 1 | |
| FORWARDREF = 2 | |
| SOURCE = 3 |
There was a problem hiding this comment.
Done (in other PR).
AA-Turner
changed the title
|
Uh, sorry folks. I forgot I created this PR last week, and re-did the work in a new PR. Should I remove the change from that PR and keep this one? Should we give up on this PR and move the conversation over there? I'm game to fix it, however best to do that. |
|
So long as #3138 contains all the changes you wish to make I think it makes sense to close this PR. A |
|
Okay, let's close this one and move the work to #3138. |
3 participants
📚 Documentation preview 📚: https://pep-previews--3127.org.readthedocs.build/