Allow dotted version strings for --python-version. #6585
Conversation
cjerdonek
added
C: download
cjerdonek
force-pushed
the
python-version-support-dotted
branch
from
88f7e06
to
85dc783
Compare
cjerdonek
force-pushed
the
python-version-support-dotted
branch
from
85dc783
to
4bb225d
Compare
| Convert a string like "3" or "34" into a tuple of ints. | ||
| Convert a version string like "3", "37", or "3.7.3" into a tuple of ints. | ||
|
|
||
| :return: A 2-tuple (version_info, error_msg), where `error_msg` is |
There was a problem hiding this comment.
This seems strange, why not raise in such case ?
There was a problem hiding this comment.
It is raised by the caller using raise_option_error() and the message returned by this function. It's easier to test this way because raise_option_error() requires both an Option and OptionParser object, and it seems like it would be an unnecessary complication to raise a ValueError just so the caller can catch it, get the message from the exception's argument, and then re-raise using raise_option_error(). (This function is used in only one place and was separated out only to make testing the --python-version argument handling inside _handle_python_version() easier.)
There was a problem hiding this comment.
It's easier to test this way
I suspected so but this return (result, error) feels strange in a python program (and looks more like golang)...
Raising an Exception would be expected (and could be tested with an assertRaise), so I guess it is a matter of taste :)
| interpreter. The version can be specified using up to three dot-separated | ||
| integers (e.g. "3" for 3.0.0, "3.7" for 3.7.0, or "3.7.3"). A major-minor | ||
| version can also be given as a string without dots (e.g. "37" for 3.7.0). | ||
| """), |
There was a problem hiding this comment.
I thought we'd want to deprecate the 37 values in favor of the unambiguous 3.7 ?
There was a problem hiding this comment.
Can that be done as a separate PR? I was hoping it could be done as part of a separate PR / discussion. I usually prefer keeping PR's smaller when possible to make it easier for people to review and discuss changes, etc (and code them :) ).
There was a problem hiding this comment.
Several PRs are fine by me
| Convert a string like "3" or "34" into a tuple of ints. | ||
| Convert a version string like "3", "37", or "3.7.3" into a tuple of ints. | ||
|
|
||
| :return: A 2-tuple (version_info, error_msg), where `error_msg` is |
There was a problem hiding this comment.
It's easier to test this way
I suspected so but this return (result, error) feels strange in a python program (and looks more like golang)...
Raising an Exception would be expected (and could be tested with an assertRaise), so I guess it is a matter of taste :)
| interpreter. The version can be specified using up to three dot-separated | ||
| integers (e.g. "3" for 3.0.0, "3.7" for 3.7.0, or "3.7.3"). A major-minor | ||
| version can also be given as a string without dots (e.g. "37" for 3.7.0). | ||
| """), |
There was a problem hiding this comment.
Several PRs are fine by me
|
Okay, thanks, @xavfernandez. |
lock
bot
added
the
auto-locked
As suggested in this comment by @xavfernandez, this PR adds support for passing dotted version strings to
--python-version: #6371 (comment)Currently, only major or major-minor version strings like
3or37are supported (with no dot).