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-31826: remove unused __version__ from mock.py by cjw296 · Pull Request #17977 · python/cpython

Conversation

@cjw296

@cjw296 cjw296 commented Jan 13, 2020 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

This isn't included in __all__ and could be a source of confusion.

https://bugs.python.org/issue31826

Sorry, something went wrong.

Uh oh!

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

All reactions
@cjw296
remove unused __version__ from mock.py
98c06c0
This isn't included in `__all__` and could be a source of confusion.
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jan 13, 2020
@bedevere-bot bedevere-bot added the awaiting core review label Jan 13, 2020
@cjw296 cjw296 added skip issue skip news labels Jan 13, 2020
@cjw296

cjw296 commented Jan 13, 2020 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

@tirkarthi / @mariocj89 - can you see any problems with this?

All reactions

Sorry, something went wrong.

Uh oh!

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

@tirkarthi

tirkarthi commented Jan 13, 2020

Copy link
Copy Markdown
Member

My username in GitHub is @tirkarthi :) Regarding the __version__ value I think it's left as it is when mock was merged to stdlib in Python 3. There is an open issue around removing __version__ for standard library at https://bugs.python.org/issue31826 and there are several notes like library being maintained externally, decimal module having it for standard etc. There are many modules with similar value like argparse that were merged to stdlib. I don't think anyone is using __version__ for this change to be a breakage. I am +0 on this.

rg "__version__ =\s['\"]\d+." Lib | grep -v '/test/'
Lib/imaplib.py:__version__ = "2.58"
Lib/optparse.py:__version__ = "1.5.3"
Lib/cgi.py:__version__ = "2.6"
Lib/argparse.py:__version__ = '1.1'
Lib/unittest/mock.py:__version__ = '1.0'
Lib/tabnanny.py:__version__ = "6"
Lib/ipaddress.py:__version__ = '1.0'
Lib/socketserver.py:__version__ = "0.4"
Lib/re.py:__version__ = "2.2.1"
Lib/_pydecimal.py:__version__ = '1.70'    # Highest version of the spec this complies with
Lib/platform.py:__version__ = '1.0.8'
Lib/logging/__init__.py:__version__ = "0.5.1.2"
Lib/json/__init__.py:__version__ = '2.0.9'
Lib/http/server.py:__version__ = "0.6"
Lib/tkinter/font.py:__version__ = "0.9"
Lib/tkinter/ttk.py:__version__ = "0.3.1"
Lib/ctypes/__init__.py:__version__ = "1.1.0"
Lib/wsgiref/simple_server.py:__version__ = "0.2"
Lib/ctypes/macholib/__init__.py:__version__ = '1.0'
All reactions

Sorry, something went wrong.

Uh oh!

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

@cjw296
cjw296 requested a review from tirkarthi January 13, 2020 19:09
@cjw296

cjw296 commented Jan 13, 2020 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

Cool, I'm comfortable removing this as you're +0. It's now confusing because it clashes with the __version__ that the backport needs to have for sensible release to pypi.

All reactions

Sorry, something went wrong.

Uh oh!

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

@cjw296
cjw296 merged commit 31d6de5 into master Jan 13, 2020
@cjw296
cjw296 deleted the cjw296-patch-1 branch January 13, 2020 19:11
@bedevere-bot

bedevere-bot commented Jan 13, 2020

Copy link
Copy Markdown

@cjw296: 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.

@bedevere-bot bedevere-bot removed the awaiting core review label Jan 13, 2020
sthagen added a commit to sthagen/python-cpython that referenced this pull request Jan 13, 2020
@sthagen
Merge pull request #176 from python/master
91f1fd3
remove unused __version__ from mock.py (python#17977)
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
@cjw296 @shihai1991
remove unused __version__ from mock.py (python#17977)
3cf3f81
This isn't included in `__all__` and could be a source of confusion.
@hroncok

hroncok commented Feb 3, 2020

Copy link
Copy Markdown
Contributor

Juts a heads up that this actually breaks things: https://bugzilla.redhat.com/show_bug.cgi?id=1797690

All reactions

Sorry, something went wrong.

Uh oh!

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

@jorti jorti mentioned this pull request Feb 3, 2020
module 'unittest.mock' has no attribute '__version__' torproject/stem#56
Closed
@cjw296

cjw296 commented Feb 3, 2020

Copy link
Copy Markdown
Contributor Author

Why are they relying on this version number? What are they using it for?

All reactions

Sorry, something went wrong.

Uh oh!

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

@hroncok

hroncok commented Feb 3, 2020

Copy link
Copy Markdown
Contributor

I have no idea. IMHO they should stop doing that, yet this change affects something at least. I suggest documenting it at https://docs.python.org/3.9/whatsnew/3.9.html

All reactions

Sorry, something went wrong.

Uh oh!

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

@cjw296

cjw296 commented Feb 3, 2020

Copy link
Copy Markdown
Contributor Author

Ah, fair, I should have done a blurb for this. No idea if I can separately put one of those in?

All reactions

Sorry, something went wrong.

Uh oh!

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

@hroncok

hroncok commented Feb 3, 2020 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Upstream issue for stem: torproject/stem#56

Ah, fair, I should have done a blurb for this. No idea if I can separately put one of those in?

No idea either. However I think that blurbs only do changelogs, https://docs.python.org/3.9/whatsnew/3.9.html is maintained separately.

cjw296 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.

@atagar

atagar commented Feb 3, 2020

Copy link
Copy Markdown

Hello, Stem's author here.

Why are they relying on this version number? What are they using it for?

Only display purposes when running the tests. Stem 1.8 supports Python 2.6-3.x, and with 2.x Mock often came from PyPI. As such the version sometimes mattered in test failures.

Is there a reason to break backward compatibility now rather than Python 4.x?

All reactions

Sorry, something went wrong.

Uh oh!

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

@cjw296

cjw296 commented Feb 3, 2020

Copy link
Copy Markdown
Contributor Author

I'm sorry, but I don't considering this a break in backwards compatibility.

This __version__ was incorrect and unmaintained. If you are using unittest.mock then the version is that of the Python distribution in use. If you are using the backport from PyPI, then it is the version of the mock package you have installed.

All reactions

Sorry, something went wrong.

Uh oh!

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

@atagar

atagar commented Feb 3, 2020

Copy link
Copy Markdown

I'm sorry, but I don't considering this a break in backwards compatibility.

Gotcha. We might need to agree to disagree on this. Python 3.6 has a version attribute whereas Python 3.9 does not, causing anyone that references it to raise an AttributeError.

Is it a useless attribute? Absolutely, and I firmly agree with removing it in Python 4.x. But as a fellow user of symantic versioning if I was in your shoes I'd flag this attribute as obsolete and remove it in Python 4.x but that said, your show. :)

All reactions

Sorry, something went wrong.

Uh oh!

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

@hroncok

hroncok commented Feb 4, 2020

Copy link
Copy Markdown
Contributor

Python regularly breaks backward compatibility in subtle ways between X.Y and X.Y+1. It does not follow semver. See also https://www.python.org/dev/peps/pep-0387/

All reactions

Sorry, something went wrong.

Uh oh!

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

@atagar

atagar commented Feb 4, 2020

Copy link
Copy Markdown

Python regularly breaks backward compatibility in subtle ways

Interesting, I didn't realize that. PEP 387 seems to advise otherwise [1] but that said, not a big whoop. Clearly this is an obscure issue and we can hotfix on our end if necessary.

[1] "Unless it is going through the deprecation process below, the behavior of an API must not change between any two consecutive releases."

All reactions

Sorry, something went wrong.

Uh oh!

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

@hroncok

hroncok commented Feb 4, 2020

Copy link
Copy Markdown
Contributor

Unless it is going through the deprecation process below, the behavior of an API must not change between any two consecutive releases.

Yes. That doesn't mean "break in 4.x", but rather "deprecate in 3.9, remove in 3.10". Neither happened here.

The confusion here IMHO is based on the fact that __version__ was not considered API. Whether or not that is reasonable assumption, I don't know. It could be deprecated via module __getattr__, if needed.

All reactions

Sorry, something went wrong.

Uh oh!

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

@encukou encukou changed the title remove unused __version__ from mock.py bpo-31826: remove unused __version__ from mock.py Feb 4, 2020
@encukou

encukou commented Feb 4, 2020 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

I took the liberty to attach this to bpo-31826.

I should have done a blurb for this. No idea if I can separately put one of those in?

You can; use bpo-31826 as the issue to link them together.

All reactions

Sorry, something went wrong.

Uh oh!

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

@abukaj abukaj mannequin mentioned this pull request May 13, 2023
Misleading __version__ attribute of modules in standard library #76007
Closed
This was referenced Oct 9, 2023
[master] Remove usage of "mock" in favor of "unittest.mock" saltstack/salt#65364
Closed
Remove usage of "mock" in favor of "unittest.mock" openSUSE/salt#606
Closed
@meaksh meaksh mentioned this pull request Oct 31, 2023
Dereference symlinks to set proper __cli opt (bsc#1215963) openSUSE/salt#611
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tirkarthi tirkarthi Awaiting requested review from tirkarthi

Uh oh!

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

Assignees

No one assigned

Labels

skip issue skip news

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.

7 participants

@cjw296 @tirkarthi @bedevere-bot @hroncok @atagar @encukou @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.