◐ Shell
reader mode source ↗
Skip to content

bpo-41428: Implementation for PEP 604#21515

Merged
pablogsal merged 62 commits into
python:masterfrom
maggiemoss:PEP605-implementation
Sep 9, 2020
Merged

bpo-41428: Implementation for PEP 604#21515
pablogsal merged 62 commits into
python:masterfrom
maggiemoss:PEP605-implementation

Conversation

@maggiemoss

@maggiemoss maggiemoss commented Jul 17, 2020 •
edited by gvanrossum
Loading

Copy link
Copy Markdown
Contributor

PEP 604 - Proposed Implementation

https://www.python.org/dev/peps/pep-0604/

Building off of the comments left on this PR here: https://github.com/pprados/cpython/tree/PEP604

https://bugs.python.org/issue41428

@maggiemoss maggiemoss requested review from a team, gvanrossum and ilevkivskyi as code owners July 17, 2020 00:07
@the-knights-who-say-ni

This comment has been minimized.

Sign in to view
@maggiemoss maggiemoss marked this pull request as draft July 17, 2020 00:07
maggiemoss referenced this pull request in pprados/cpython Jul 17, 2020
@pprados
Implement a POC of the specification describe in the PEP0604
e3e99bc
@maggiemoss maggiemoss changed the title Pep605 implementation Jul 17, 2020
@gvanrossum

gvanrossum commented Jul 17, 2020

Copy link
Copy Markdown
Member

Thank you so much! Give me a few days to review this. Ping me if you haven't heard from me by Wednesday!

@gvanrossum gvanrossum changed the title Pep604 - Draft Implementation Jul 18, 2020
gvanrossum
gvanrossum reviewed Jul 18, 2020
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

Great start! Here's a boatload of review comments, from whitespace nits to feature requests.

13 hidden conversations Load more…
gvanrossum
gvanrossum reviewed Jul 19, 2020
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

(Presumably you were going to add more commits before asking for another review?)

@maggiemoss

maggiemoss commented Jul 21, 2020

Copy link
Copy Markdown
Contributor Author

@gvanrossum thanks so much for the comments! I'll implement your suggestions and clean this up :)

129 hidden items Load more…
@maggiemoss maggiemoss requested a review from pablogsal August 18, 2020 22:32
@maggiemoss

maggiemoss commented Aug 20, 2020

Copy link
Copy Markdown
Contributor Author

@gvanrossum

I had thought of another case we might need to handle here, and was hoping to get your input. Should this:

typing.Union[int, str] is (int | str)

evaluate to true?

@maggiemoss

maggiemoss commented Aug 23, 2020

Copy link
Copy Markdown
Contributor Author

@pablogsal let me know if there are any other changes you'd like to see here.

@pablogsal

pablogsal commented Aug 23, 2020

Copy link
Copy Markdown
Member

@pablogsal let me know if there are any other changes you'd like to see here.

I just need to find some time to do another iteration 😉 Will try to do another pass by tomorrow, but we are very close! Thanks for the patience and tye good work :)

@pablogsal

pablogsal commented Sep 8, 2020

Copy link
Copy Markdown
Member

@maggiemoss I have pushed a new commit (8c06c1d) to do some simplifications and to fix some minor reference count issues and crashes. Please, check out that everything seems ok in that commit :)

pablogsal
pablogsal approved these changes Sep 9, 2020
View reviewed changes

@pablogsal pablogsal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

This looks good to me!

Thanks for all the work and patience @maggiemoss! Great job :)

@gvanrossum I won't land it myself in case you or Maggie want to take a last check.

gvanrossum
gvanrossum approved these changes Sep 9, 2020
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

I think it's good! Thanks so much Maggie for all your work.

@maggiemoss

maggiemoss commented Sep 9, 2020

Copy link
Copy Markdown
Contributor Author

Thanks so much for all of your reviews and comments, I've really appreciated it. 🎉

@gvanrossum

gvanrossum commented Sep 9, 2020

Copy link
Copy Markdown
Member

@maggiemoss
I noticed that some tests failed on Windows. Could it be that you need to rebase to the latest master?

"test.test_asyncio.test_sendfile.ProactorEventLoopTests.test_sendfile_close_peer_in_the_middle_of_receiving"

@pablogsal

pablogsal commented Sep 9, 2020

Copy link
Copy Markdown
Member

@maggiemoss
I noticed that some tests failed on Windows. Could it be that you need to rebase to the latest master?

"test.test_asyncio.test_sendfile.ProactorEventLoopTests.test_sendfile_close_peer_in_the_middle_of_receiving"

That's an unrelated race condition in asyncio in Windows, we can ignore it and just land

@pablogsal pablogsal merged commit 1b4552c into python:master Sep 9, 2020
@gvanrossum

gvanrossum commented Sep 9, 2020 via email

Copy link
Copy Markdown
Member

xzy3 pushed a commit to xzy3/cpython that referenced this pull request Oct 18, 2020
@maggiemoss @pablogsal @xzy3-cdc
bpo-41428: Implementation for PEP 604 (pythonGH-21515) …
293703d 
See https://www.python.org/dev/peps/pep-0604/ for more information.

Co-authored-by: Pablo Galindo <pablogsal@gmail.com>
@Fidget-Spinner Fidget-Spinner mentioned this pull request Jul 19, 2021
Merged
AlexWaygood added a commit to AlexWaygood/typeshed that referenced this pull request Dec 4, 2021
@AlexWaygood
Add __(r)or__ to various typing classes …
2794d7f 
- `__or__` was added to `TypeVar` in Python 3.10: https://bugs.python.org/issue41428 (this PR: python/cpython#21515)
- `__or__` was added to `ForwardRef` in Python 3.11: https://bugs.python.org/issue45489
JelleZijlstra pushed a commit to python/typeshed that referenced this pull request Dec 5, 2021
@AlexWaygood
Add __(r)or__ to various typing classes (#6498) …
ff4bd7c 
- `__or__` was added to `TypeVar` in Python 3.10: https://bugs.python.org/issue41428 (this PR: python/cpython#21515)
- `__or__` was added to `ForwardRef` in Python 3.11: https://bugs.python.org/issue45489
@furkanonder furkanonder mentioned this pull request Mar 28, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pablogsal pablogsal

@gvanrossum gvanrossum gvanrossum approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@maggiemoss @the-knights-who-say-ni @gvanrossum @pablogsal @bedevere-bot