bpo-24916: Change the way to get the python version. by damani42 · Pull Request #18487 · python/cpython (original) (raw)
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Use sys.version_info instead sys.version to get the python version.
Hello, and thanks for your contribution!
I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).
CLA Missing
Our records indicate the following people have not signed the CLA:
For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.
If you have recently signed the CLA, please wait at least one business day
before our records are updated.
You can check yourself to see if the CLA has been received.
Thanks again for the contribution, we look forward to reviewing it!
| py_version = sys.version.split()[0] |
|---|
| py_version = '{major}.{minor}.{micro}'.format( |
| major=sys.version_info.major, minor=sys.version_info.minor, |
| micro=sys.version_info.micro) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may use this shorter syntax:
py_version = '{0.major}.{0.minor}.{0.micro}'.format(sys.version_info)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or shorter:
py_version = '%s.%s.%s' % sys.version_info[:3]
But there was a reason why this code still uses sys.version while other sites were changed to use sys.version_info. sys.version contains some details not available in sys.version_info. Since this field is only purposed to output to humans, this is the right usage of sys.version.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, thanks for the contribution @damani42!
Apart from the better syntax @vstinner suggested, there is another problem as well. Correct me if I'm wrong, but, before this patch, py_version here would be something like 3.8.2rc1+ for 3.8, but it is just 3.8.2 after the patch, which is incorrect.
Apart from the better syntax @vstinner suggested, there is another problem as well. Correct me if I'm wrong, but, before this patch, py_version here would be something like 3.8.2rc1+ for 3.8, but it is just 3.8.2 after the patch, which is incorrect.
py_version is currently unused. It's a variable which might be used in a scheme to build a path. Is it really a good idea to put "a3" or "+" in a path?
Example in master:
>>> import sysconfig; sysconfig._PY_VERSION
'3.9.0a3+'
Another simpler approach but backward incompatible would be to remove the "py_version" variable from sysconfig (and distutils.command.install).
Does anyone know if it's used in the wild?
Another simpler approach but backward incompatible would be to remove the "py_version" variable from sysconfig (and distutils.command.install).
In case this isn't used out in the wild, this seems like the best approach to me, too.
farazs-github pushed a commit to MediaTek-Labs/cpython that referenced this pull request
use content of pybuilddir.txt to find just build extentions