Skip to content

/ cpython

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

Jump to bottom

bpo-44662: Add ability to annotate types.Union #27214

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

bpo-44662: Add ability to annotate types.Union #27214

wants to merge 8 commits into from
+38 −1

Conversation

@uriyyo
Copy link
Contributor

@uriyyo uriyyo commented Jul 17, 2021 •
edited

https://bugs.python.org/issue44662
@uriyyo uriyyo requested a review from gvanrossum as a code owner Jul 17, 2021
@uriyyo
Copy link
Contributor Author

@uriyyo uriyyo commented Jul 17, 2021

@Fidget-Spinner Could you please review this PR?
@Fidget-Spinner
Fidget-Spinner reviewed Jul 17, 2021
View changes
Copy link
Contributor

@Fidget-Spinner Fidget-Spinner left a comment •
edited

Please wait until Serhiy merges some of his union refactoring changes. He's already prepared them, just not yet made PRs for review and merge. This will also take some time for me to review as I need to revise on tp_getattro and friends. Sorry (I'm busy with other things at the moment).

Also the current issue is getting too big. Can you please create a separate issue for this on bpo then tie this PR to that? We are getting pretty far from my original issue on the bpo ;).
Objects/unionobject.c Outdated Show resolved Hide resolved
@uriyyo uriyyo changed the title bpo-44490: Add ability to serialise and annotate types.Union bpo-44662: Add ability to serialise and annotate types.Union Jul 17, 2021
@uriyyo
Copy link
Contributor Author

@uriyyo uriyyo commented Jul 17, 2021

I created separate issue at issue tracker.
uriyyo added 3 commits Jul 17, 2021
@uriyyo
bpo-44490: Add ability to serialise and annotate types.Union
5021ca7
@uriyyo
Update news
43556fe
@uriyyo
Update unionobject
eb52dc8
@uriyyo uriyyo force-pushed the uriyyo:fix-issue-44490 branch from 97a1898 to eb52dc8 Jul 19, 2021
@uriyyo uriyyo requested a review from Fidget-Spinner Jul 19, 2021
@uriyyo
Copy link
Contributor Author

@uriyyo uriyyo commented Jul 19, 2021

@Fidget-Spinner I have merged Serhiy changes. Could you please review this PR?
@Fidget-Spinner
Copy link
Contributor

@Fidget-Spinner Fidget-Spinner commented Jul 19, 2021

Can you please split the PR into two? It seems it's fixing pickling and typing.Annotated compatibility at the same time. Please create another issue for the annotated problem.
@uriyyo
Copy link
Contributor Author

@uriyyo uriyyo commented Jul 19, 2021

Sorry about that, I thought that it will be a great idea to fix several issues in scope of one PR.

I will divide it at 2 separate PR.
@uriyyo uriyyo changed the title bpo-44662: Add ability to serialise and annotate types.Union bpo-44662: Add ability to annotate types.Union Jul 19, 2021
uriyyo added 3 commits Jul 19, 2021
@uriyyo
Remove Union serialisation code
2fcfafe
@uriyyo
Fix _strip_annotations
65558f2
@uriyyo
Remove whitespace
8a13208
@uriyyo
Copy link
Contributor Author

@uriyyo uriyyo commented Jul 19, 2021

@Fidget-Spinner This PR contains only typing.Annotated compatibility fix.
@Fidget-Spinner
Copy link
Contributor

@Fidget-Spinner Fidget-Spinner commented Jul 19, 2021

Sorry about that, I thought that it will be a great idea to fix several issues in scope of one PR.

I will divide it at 2 separate PR.

No worries. Thanks for separating them. It's much clearer to me now.
@Fidget-Spinner
Fidget-Spinner reviewed Jul 19, 2021
View changes
Copy link
Contributor

@Fidget-Spinner Fidget-Spinner left a comment

Looks good, I'll nosy a few others in too in case I missed something.
Objects/unionobject.c Outdated Show resolved Hide resolved
Misc/NEWS.d/next/Core and Builtins/2021-07-17-13-41-58.bpo-44662.q22kWR.rst Outdated Show resolved Hide resolved
uriyyo and others added 2 commits Jul 19, 2021
@uriyyo @Fidget-Spinner
Update Misc/NEWS.d/next/Core and Builtins/2021-07-17-13-41-58.bpo-44662…
36db2f5 
….q22kWR.rst

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
@uriyyo
Add PyUnicode_Check to union_getattro
136f81c
@uriyyo uriyyo requested a review from Fidget-Spinner Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gvanrossum gvanrossum

@Fidget-Spinner Fidget-Spinner

Assignees
No one assigned
Labels
CLA signed awaiting review
Projects
None yet
Milestone
No milestone
4 participants
@uriyyo @Fidget-Spinner @the-knights-who-say-ni @bedevere-bot