GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-44732: Rename types.Union to types.UnionType by AliyevH · Pull Request #27342 · python/cpython

Conversation

@AliyevH

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

https://bugs.python.org/issue44732

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@AliyevH
AliyevH requested a review from gvanrossum as a code owner July 24, 2021 23:53
@bedevere-bot bedevere-bot added the awaiting review label Jul 24, 2021
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jul 24, 2021
FFY00
FFY00 requested changes Jul 25, 2021
View reviewed changes

@FFY00 FFY00 left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

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}')

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Jul 25, 2021
FFY00
FFY00 reviewed Jul 25, 2021
View reviewed changes
Comment thread Misc/NEWS.d/next/Core and Builtins/2021-07-19-19-53-46.bpo-44676.WgIMvh.rst Outdated

@FFY00 FFY00 Jul 25, 2021

Copy link
Copy Markdown
Member

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.

We should not change this news entry 😛

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@AliyevH AliyevH Jul 25, 2021

Copy link
Copy Markdown
Contributor Author

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.

Didn't know about it 😕

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@FFY00 FFY00 Jul 25, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@AliyevH AliyevH Jul 25, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@FFY00 FFY00 Jul 25, 2021

Copy link
Copy Markdown
Member

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@AliyevH AliyevH Jul 25, 2021

Copy link
Copy Markdown
Contributor Author

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.

Okey thanks 😄

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@Fidget-Spinner

Fidget-Spinner commented Jul 25, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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!

FFY00 and AliyevH reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AliyevH

AliyevH commented Jul 25, 2021

Copy link
Copy Markdown
Contributor Author

Thanks @Fidget-Spinner . Okey let's wait 😃

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka
serhiy-storchaka approved these changes Jul 25, 2021
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

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.

LGTM!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Jul 25, 2021
@serhiy-storchaka serhiy-storchaka added the needs backport to 3.10 label Jul 25, 2021
@serhiy-storchaka
serhiy-storchaka requested a review from pablogsal July 25, 2021 20:23
@JelleZijlstra

JelleZijlstra commented Jul 26, 2021

Copy link
Copy Markdown
Member

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 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum

gvanrossum commented Jul 26, 2021

Copy link
Copy Markdown
Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AliyevH

AliyevH commented Jul 26, 2021

Copy link
Copy Markdown
Contributor Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

uriyyo
uriyyo reviewed Jul 26, 2021
View reviewed changes
Comment thread Lib/types.py Outdated

@uriyyo uriyyo Jul 26, 2021

Copy link
Copy Markdown
Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@AliyevH AliyevH Jul 26, 2021

Copy link
Copy Markdown
Contributor Author

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.

No :) I forgot about it

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@AliyevH AliyevH Jul 26, 2021

Copy link
Copy Markdown
Contributor Author

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.

Fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@AliyevH
bpo-44732: Rename types.Union to types.UnionType
1dc1ff0
uriyyo
uriyyo approved these changes Jul 26, 2021
View reviewed changes

@uriyyo uriyyo left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

LGTM💫

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Fidget-Spinner
Fidget-Spinner approved these changes Jul 26, 2021
View reviewed changes

@Fidget-Spinner Fidget-Spinner left a comment

Copy link
Copy Markdown
Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Fidget-Spinner
Fidget-Spinner requested changes Jul 26, 2021
View reviewed changes

@Fidget-Spinner Fidget-Spinner left a comment

Copy link
Copy Markdown
Member

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.

Wait, the docs here need updating too https://docs.python.org/3.10/library/types.html#types.Union.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AliyevH reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@AliyevH
bpo-44732: Updated Union to UnionType in doc
a21c490
@AliyevH

AliyevH commented Jul 26, 2021

Copy link
Copy Markdown
Contributor Author

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 )

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra
JelleZijlstra reviewed Jul 26, 2021
View reviewed changes
Comment thread Misc/NEWS.d/next/Core and Builtins/2021-07-26-15-27-03.bpo-44732.IxObt3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@ambv @JelleZijlstra
Make the NEWS entry a full sentence
3c35f62
Co-authored-by: Jelle Zijlstra <[email protected]>
@ambv
ambv merged commit 2b8ad9e into python:main Jul 26, 2021
@bedevere-bot

bedevere-bot commented Jul 26, 2021

Copy link
Copy Markdown

@ambv: Please replace # with GH- in the commit message next time. Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington

miss-islington commented Jul 26, 2021

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the awaiting merge label Jul 26, 2021
@bedevere-bot bedevere-bot removed the needs backport to 3.10 label Jul 26, 2021
@bedevere-bot

bedevere-bot commented Jul 26, 2021

Copy link
Copy Markdown

GH-27365 is a backport of this pull request to the 3.10 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington pushed 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 <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
(cherry picked from commit 2b8ad9e)

Co-authored-by: Hasan <[email protected]>
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 <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
(cherry picked from commit 2b8ad9e)

Co-authored-by: Hasan <[email protected]>
@tk0miya tk0miya mentioned this pull request Jul 29, 2021
types.Union was renamed to types.UnionType on the HEAD of 3.10 sphinx-doc/sphinx#9512
Closed
tk0miya added a commit to tk0miya/sphinx that referenced this pull request Jul 29, 2021
@tk0miya
Fix sphinx-doc#9512: sphinx-build: crashed with the HEAD of Python 3.10
e48865c
Recently, `types.Union` was renamed to `types.UnionType` on the HEAD
of 3.10 (refs: python/cpython#27342). After this change, sphinx-build
has been crashed because of ImportError.
@tk0miya tk0miya mentioned this pull request Jul 29, 2021
Fix #9512: sphinx-build: crashed with the HEAD of Python 3.10 sphinx-doc/sphinx#9513
Merged
tk0miya added a commit to tk0miya/sphinx that referenced this pull request Jul 29, 2021
@tk0miya
Fix sphinx-doc#9512: sphinx-build: crashed with the HEAD of Python 3.10
8b2031c
Recently, `types.Union` was renamed to `types.UnionType` on the HEAD
of 3.10 (refs: python/cpython#27342). After this change, sphinx-build
has been crashed because of ImportError.
tk0miya added a commit to sphinx-doc/sphinx that referenced this pull request Jul 29, 2021
@tk0miya
Fix #9512: sphinx-build: crashed with the HEAD of Python 3.10
1fd5f74
Recently, `types.Union` was renamed to `types.UnionType` on the HEAD
of 3.10 (refs: python/cpython#27342). After this change, sphinx-build
has been crashed because of ImportError.
@orsenthil

orsenthil commented Aug 12, 2021

Copy link
Copy Markdown
Member

As it is already referenced in this ticket, it broke (the stable version of )sphinx-build and multiple documentation projects are going to be affected by this change.

This is the trackback from sphinx-build command line.

  File "/Users/senthilx/venvs/cpython/bin/sphinx-quickstart", line 5, in <module>
    from sphinx.cmd.quickstart import main
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/senthilx/venvs/cpython/lib/python3.11/site-packages/sphinx/cmd/quickstart.py", line 38, in <module>
    from sphinx.util.console import bold, color_terminal, colorize, nocolor, red  # type: ignore
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/senthilx/venvs/cpython/lib/python3.11/site-packages/sphinx/util/__init__.py", line 41, in <module>
    from sphinx.util.typing import PathMatcher
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/senthilx/venvs/cpython/lib/python3.11/site-packages/sphinx/util/typing.py", line 37, in <module>
    from types import Union as types_Union
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ImportError: cannot import name 'Union' from 'types' (/Users/senthilx/git/cpython/Lib/types.py)

Sphinx has already fixed it in master (sphinx-doc/sphinx#9513) - The fix (aliasing to UnionType) doesn't seem great too.

If we get more complaints, I think, either CPython or Sphinx project need to address this by communication / guidance.

It was hard to track down this failure with the trace back.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Fidget-Spinner

Fidget-Spinner commented Aug 12, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

@orsenthil that's sad to hear :(. Is there some better place to make such changes known to the wider community?

This change was discussed and agreed upon on python-dev, but that's centered around CPython so I don't think there's much visibility. It also doesn't get a What's New because it was a change during pre-release versions.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@skshetry skshetry mentioned this pull request Aug 13, 2021
bump Scheduled weekly dependency update for week 32 skshetry/webdav4#48
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments
@FFY00 FFY00 FFY00 requested changes
@Fidget-Spinner Fidget-Spinner Fidget-Spinner requested changes
@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes
@uriyyo uriyyo uriyyo approved these changes
@gvanrossum gvanrossum Awaiting requested review from gvanrossum
@pablogsal pablogsal Awaiting requested review from pablogsal

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

12 participants

@AliyevH @Fidget-Spinner @JelleZijlstra @gvanrossum @bedevere-bot @miss-islington @orsenthil @serhiy-storchaka @FFY00 @uriyyo @ambv @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.