Skip to content

bpo-31826: remove unused __version__ from mock.py #17977

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

Merged
merged 1 commit into from
Jan 13, 2020
Merged

Conversation

cjw296
Copy link
Contributor

@cjw296 cjw296 commented Jan 13, 2020

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

https://bugs.python.org/issue31826

This isn't included in `__all__` and could be a source of confusion.
@cjw296
Copy link
Contributor Author

cjw296 commented Jan 13, 2020

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

@tirkarthi
Copy link
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'

@cjw296 cjw296 requested a review from tirkarthi January 13, 2020 19:09
@cjw296
Copy link
Contributor Author

cjw296 commented Jan 13, 2020

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.

@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
Copy link

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

sthagen added a commit to sthagen/python-cpython that referenced this pull request Jan 13, 2020
remove unused __version__ from mock.py (python#17977)
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
This isn't included in `__all__` and could be a source of confusion.
@hroncok
Copy link
Contributor

hroncok commented Feb 3, 2020

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

@cjw296
Copy link
Contributor Author

cjw296 commented Feb 3, 2020

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

@hroncok
Copy link
Contributor

hroncok commented Feb 3, 2020

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

@cjw296
Copy link
Contributor Author

cjw296 commented Feb 3, 2020

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

@hroncok
Copy link
Contributor

hroncok commented Feb 3, 2020

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.

@atagar
Copy link

atagar commented Feb 3, 2020

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?

@cjw296
Copy link
Contributor Author

cjw296 commented Feb 3, 2020

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.

@atagar
Copy link

atagar commented Feb 3, 2020

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. :)

@hroncok
Copy link
Contributor

hroncok commented Feb 4, 2020

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/

@atagar
Copy link

atagar commented Feb 4, 2020

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

@hroncok
Copy link
Contributor

hroncok commented Feb 4, 2020

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.

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

encukou commented Feb 4, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants