-
-
Notifications
You must be signed in to change notification settings - Fork 30.7k
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
bpo-42128: Update pattern matching docs to match new PEP changes #25185
bpo-42128: Update pattern matching docs to match new PEP changes #25185
Conversation
@@ -1057,7 +1057,7 @@ subject value: | |||
|
|||
* If this raises an exception, the exception bubbles up. | |||
|
|||
* If the returned value is not a list or tuple, the conversion fails and |
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.
There's some discussion on putting this check when the attribute is set rather than looked up. I wouldn't change this until this is decided
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.
Yeah I agree. I'm waiting for the other two PRs in the PEP repo to get finalised and merged. Would be nice if someone added the DO-NOT-MERGE
label to this PR temporarily (I'm unable to do so).
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.
Would be nice if someone added the
DO-NOT-MERGE
label to this PR temporarily (I'm unable to do so).
I turned it into draft mode instead (you should be able to un-draft it yourself -- let me know if you can't).
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.
Well I'm feeling a little silly now - I forgot about drafts. Thanks Guido! (I'm also able to un-draft it myself)
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.
There's some discussion on putting this check when the attribute is set rather than looked up. I wouldn't change this until this is decided
I don't think we'll change when the check is done right away, but we're changing the acceptable type to tuple right away (#25203), so the docs need to be changed. At what point the check is done would affect the tiniest fraction of all code, but disallowing lists might already affect existing (bleeding edge) code.
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.
You can un-draft this now!
@@ -1057,7 +1057,7 @@ subject value: | |||
|
|||
* If this raises an exception, the exception bubbles up. | |||
|
|||
* If the returned value is not a list or tuple, the conversion fails and |
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.
There's some discussion on putting this check when the attribute is set rather than looked up. I wouldn't change this until this is decided
I don't think we'll change when the check is done right away, but we're changing the acceptable type to tuple right away (#25203), so the docs need to be changed. At what point the check is done would affect the tiniest fraction of all code, but disallowing lists might already affect existing (bleeding edge) code.
@gvanrossum: Please replace |
On python-dev, changes to PEP 634 were discussed and proposed, this patch just brings the docs up to speed:
__match_args__
must be a tupleRef: https://mail.python.org/archives/list/python-dev@python.org/thread/YYIT3QXMLPNLXQAQ5BCXE4LLJ57EE7JV/
python/peps#1908 and python/peps#1909 needs to be merged before this can be merged.
https://bugs.python.org/issue42128