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-44732: Rename types.Union to types.UnionType #27342

Merged
merged 3 commits into from Jul 26, 2021
Merged

bpo-44732: Rename types.Union to types.UnionType #27342

merged 3 commits into from Jul 26, 2021
+22 −22

Conversation

@AliyevH
Copy link
Contributor

@AliyevH AliyevH commented Jul 24, 2021 •
edited by bedevere-bot

https://bugs.python.org/issue44732
@AliyevH AliyevH requested a review from gvanrossum as a code owner Jul 24, 2021
@AliyevH AliyevH force-pushed the AliyevH:main branch from 53aed25 to 3357489 Jul 24, 2021
@FFY00
FFY00 suggested changes Jul 25, 2021
View changes
Copy link
Member

@FFY00 FFY00 left a comment •
edited

A plain rename from Union to UnionType would be a big breaking change, it should be deprecated IMO.

We could do that in types.__getattr__, by adding the following to the types module.

import warnings

...

def __getattr__(name):
    if name == 'Union':
        warnings.warn('types.Union is deprecated and should be replaced with types.UnionType, it will get removed in X time', DeprecationWarning)
        return UnionType
    raise AttributeError(f'module {__name__} has no attribute {name}')
@FFY00
FFY00 reviewed Jul 25, 2021
View changes
Misc/NEWS.d/next/Core and Builtins/2021-07-19-19-53-46.bpo-44676.WgIMvh.rst Outdated
@@ -1,2 +1,2 @@
Add ability to serialise ``types.Union`` objects. Patch provided by Yurii
Add ability to serialise ``types.UnionType`` objects. Patch provided by Yurii

This comment has been minimized.

Sign in to view
@FFY00

FFY00 Jul 25, 2021
Member

We should not change this news entry 😛

This comment has been minimized.

Sign in to view
@AliyevH

AliyevH Jul 25, 2021
Author Contributor

Didn't know about it 😕

This comment has been minimized.

Sign in to view
@FFY00

FFY00 Jul 25, 2021 •
edited
Member

Oh wait, maybe we do actually want this changed as it hasn't been released yet 🤦. Sorry!

This comment has been minimized.

Sign in to view
@AliyevH

AliyevH Jul 25, 2021 •
edited
Author Contributor

no problem.
Can you please tell me X time in deprecation warning?😬

This comment has been minimized.

Sign in to view
@FFY00

FFY00 Jul 25, 2021
Member

I don't know, that's up to the core devs, ask in https://bugs.python.org/issue44732 if we should have a deprecation period and if so, how long.

This comment has been minimized.

Sign in to view
@AliyevH

AliyevH Jul 25, 2021
Author Contributor

Okey thanks 😄
@Fidget-Spinner
Copy link
Contributor

@Fidget-Spinner Fidget-Spinner commented Jul 25, 2021 •
edited

A plain rename from Union to UnionType would be a big breaking change, it should be deprecated IMO.

From what I understand, it won't be. It looks like Serhiy is planning to backport the changes to 3.10 too where Union was first introduced. Hasan, please wait a bit to see what he has to say about the PR. Thanks!
@AliyevH
Copy link
Contributor Author

@AliyevH AliyevH commented Jul 25, 2021

Thanks @Fidget-Spinner . Okey let's wait 😃
@serhiy-storchaka
serhiy-storchaka approved these changes Jul 25, 2021
View changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

LGTM!
@serhiy-storchaka serhiy-storchaka requested a review from pablogsal Jul 25, 2021
@JelleZijlstra
Copy link
Member

@JelleZijlstra JelleZijlstra commented Jul 26, 2021

I don't see a NEWS entry in the PR but bedevere passed, am I missing something?

I do think there should be a NEWS entry for this.
@gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Jul 26, 2021

I don't see a NEWS entry in the PR but bedevere passed, am I missing something?

I do think there should be a NEWS entry for this.

The OP updated all of the existing NEWS entries that mention types.Union. That's creative, but I don't think it should be done that way. We need a new NEWS entry describing the renaming.
@AliyevH
Copy link
Contributor Author

@AliyevH AliyevH commented Jul 26, 2021

As i understand, we don't touch news files and add a new one with a description of renaming types.Union to types.UnionType.
I will update this today.
@AliyevH AliyevH force-pushed the AliyevH:main branch from 3357489 to 36a7b7a Jul 26, 2021
@uriyyo
uriyyo reviewed Jul 26, 2021
View changes
Lib/types.py Outdated
@@ -2,6 +2,7 @@
Define names for built-in types that aren't directly accessible as a builtin.
"""
import sys
import warnings

This comment has been minimized.

Sign in to view
@uriyyo

uriyyo Jul 26, 2021
Contributor

Do we need import warnings? I can not see the usage of this module at types.py.

This comment has been minimized.

Sign in to view
@AliyevH

AliyevH Jul 26, 2021
Author Contributor

No :) I forgot about it

This comment has been minimized.

Sign in to view
@AliyevH

AliyevH Jul 26, 2021
Author Contributor

Fixed
@AliyevH
bpo-44732: Rename types.Union to types.UnionType
1dc1ff0
@AliyevH AliyevH force-pushed the AliyevH:main branch from 36a7b7a to 1dc1ff0 Jul 26, 2021
@uriyyo
uriyyo approved these changes Jul 26, 2021
View changes
Copy link
Contributor

@uriyyo uriyyo left a comment •
edited

LGTM💫
@Fidget-Spinner
Fidget-Spinner approved these changes Jul 26, 2021
View changes
Copy link
Contributor

@Fidget-Spinner Fidget-Spinner left a comment

LGTM. Thanks for your contribution!

Minor note: in the future please don't force-push as it overrides commit history @AliyevH. Just commit normally, the core devs will squash and merge the PR.

https://devguide.python.org/pullrequest/#quick-guide
@Fidget-Spinner
Fidget-Spinner suggested changes Jul 26, 2021
View changes
Copy link
Contributor

@Fidget-Spinner Fidget-Spinner left a comment

Wait, the docs here need updating too https://docs.python.org/3.10/library/types.html#types.Union.
@AliyevH
bpo-44732: Updated Union to UnionType in doc
a21c490
@AliyevH
Copy link
Contributor Author

@AliyevH AliyevH commented Jul 26, 2021

cs he

LGTM. Thanks for your contribution!

Minor note: in the future please don't force-push as it overrides commit history @AliyevH. Just commit normally, the core devs will squash and merge the PR.

https://devguide.python.org/pullrequest/#quick-guide

Got it. Thanks for support )
@JelleZijlstra
JelleZijlstra reviewed Jul 26, 2021
View changes
Misc/NEWS.d/next/Core and Builtins/2021-07-26-15-27-03.bpo-44732.IxObt3.rst Outdated Show resolved Hide resolved
@ambv @JelleZijlstra
Make the NEWS entry a full sentence
3c35f62 
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@ambv ambv merged commit 2b8ad9e into python:main Jul 26, 2021
13 checks passed
13 checks passed
@github-actions
Docs
Details
@github-actions
Check for source changes
Details
@github-actions
Check if generated files are up to date
Details
@github-actions
Windows (x86)
Details
@github-actions
Windows (x64)
Details
@github-actions
macOS
Details
@github-actions
Ubuntu
Details
@github-actions
Ubuntu SSL tests with OpenSSL
Details
@github-actions
Address sanitizer Address sanitizer
Details
Azure Pipelines PR #20210726.19 succeeded
Details
@travis-ci
Travis CI - Pull Request Build Passed
Details
@bedevere-bot
bedevere/issue-number Issue number 44732 found
Details
@bedevere-bot
bedevere/news News entry found in Misc/NEWS.d
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Jul 26, 2021

@ambv: Please replace # with GH- in the commit message next time. Thanks!
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Jul 26, 2021

Thanks @AliyevH for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Jul 26, 2021

GH-27365 is a backport of this pull request to the 3.10 branch.
miss-islington added a commit to miss-islington/cpython that referenced this pull request Jul 26, 2021
@AliyevH @miss-islington
bpo-44732: Rename types.Union to types.UnionType (pythonGH-27342)
d00d0c7 
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
(cherry picked from commit 2b8ad9e)

Co-authored-by: Hasan <hasan.aleeyev@gmail.com>
miss-islington added a commit that referenced this pull request Jul 26, 2021
@miss-islington @AliyevH
bpo-44732: Rename types.Union to types.UnionType (GH-27342)
8a37e8c 
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
(cherry picked from commit 2b8ad9e)

Co-authored-by: Hasan <hasan.aleeyev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JelleZijlstra JelleZijlstra

@FFY00 FFY00

@Fidget-Spinner Fidget-Spinner

@serhiy-storchaka serhiy-storchaka

@uriyyo uriyyo

@gvanrossum gvanrossum

@pablogsal pablogsal

Assignees
No one assigned
Labels
CLA signed
Projects
None yet
Milestone
No milestone