-
Notifications
You must be signed in to change notification settings - Fork 256
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
tags: Separate versions with multidigit components with '_' #240
Conversation
@jwodder just an FYI this PR is going to have massive merge conflicts once #231 lands (which should hopefully be in the very near future). I also don't know yet about the proposed change. There hasn't been a discussion about how to handle Python 3.10 so I don't know how we may want to solve it. It may be that ripping out any custom version number specification for CPython is best and to just entirely rely on |
PR updated for #231. |
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.
Just some ideas on how to simplify the tests.
tests/test_tags.py
Outdated
@@ -543,7 +553,11 @@ def test_wide_unicode(self, unicode_size, maxunicode, version, result, monkeypat | |||
config = {"Py_DEBUG": 0, "WITH_PYMALLOC": 0, "Py_UNICODE_SIZE": unicode_size} | |||
monkeypatch.setattr(sysconfig, "get_config_var", config.__getitem__) | |||
monkeypatch.setattr(sys, "maxunicode", maxunicode) | |||
base_abi = "cp{}{}".format(version[0], version[1]) | |||
if version[0] >= 10 or version[1] >= 10: |
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.
It's okay to call _version_nodot()
in this case as you're not testing the version details, so no need to duplicate the logic here.
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.
The tests now use _version_nodot()
where applicable.
tests/test_tags.py
Outdated
@@ -596,7 +643,11 @@ def test_all_args(self): | |||
|
|||
def test_python_version_defaults(self): | |||
tag = next(tags.cpython_tags(abis=["abi3"], platforms=["any"])) | |||
interpreter = "cp{}{}".format(*sys.version_info[:2]) | |||
if sys.version_info[0] >= 10 or sys.version_info[1] >= 10: |
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.
You can call _version_nodot()
directly as the test is just making sure the result is based on sys.version_info
and is formatted appropriately.
tests/test_tags.py
Outdated
@@ -806,13 +955,23 @@ def test_windows_cpython(self, mock_interpreter_name, monkeypatch): | |||
abis = tags._cpython_abis(sys.version_info[:2]) | |||
platforms = tags._generic_platforms() | |||
result = list(tags.sys_tags()) | |||
interpreter = "cp{major}{minor}".format( | |||
major=sys.version_info[0], minor=sys.version_info[1] | |||
if sys.version_info[0] >= 10 or sys.version_info[1] >= 10: |
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.
For the cases where it's worth keeping the logic it should probably be broken out into its own test function. Although honestly unless the test is specifically for formatting and thus has hard-coded expectations I say just call _version_nodot()
directly.
@jwodder BTW when you're ready for another review just refresh the review request and it will show up in my GitHub PR review queue. |
Thanks! |
PEP 425 defines the version string used in Python implementation tags as follows:
I take this to mean that, starting with Python 3.10, Python implementation tags will be of the form
cp3_10
instead ofcp310
. This PR thus updatestags.py
to insert underscores in version strings in tags whenever one or more of the version's components are at least 10.Note: I am assuming that the sysconfig var
py_version_nodot
will be updated to use an underscore when Python 3.10 is released; if it is not, then the definition ofinterpreter_version()
will have to be changed to use something other thanpy_version_nodot
.