-
-
Notifications
You must be signed in to change notification settings - Fork 69
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
Split parsing of Poetry versions into a separate module #557
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
9388889
Fix type hints
maresb 513aec0
Split nested function calls in coerce_to_semver
maresb 6fb7fa2
Add more tests to fail on invalid semver
maresb 780dcf4
Fix semver regex
maresb 1704957
Split history py_toml.py to parse_poetry_version.py - rename file to …
maresb 3aab8ff
Split history py_toml.py to parse_poetry_version.py - rename source-f…
maresb cc83bb5
Split history py_toml.py to parse_poetry_version.py - resolve conflic…
maresb 111e396
Split history py_toml.py to parse_poetry_version.py - restore name of…
maresb 539e0f7
Complete the splitoff into parse_poetry_version
maresb fa9dacb
Split history test_py_toml.py to test_parse_poetry_version.py - renam…
maresb d13b90b
Split history test_py_toml.py to test_parse_poetry_version.py - renam…
maresb 8345ed0
Split history test_py_toml.py to test_parse_poetry_version.py - resol…
maresb 48c512b
Split history test_py_toml.py to test_parse_poetry_version.py - resto…
maresb c0a63a1
Complete the splitoff of the tests
maresb 0736b95
Convert most parse_poetry_version tests to doctests
maresb bdd0393
Update failing CRAN test
maresb 6954797
Enable doctests
maresb 1432f07
Merge branch 'main' into copy-py-toml
maresb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,224 @@ | ||
import re | ||
from typing import Dict, Optional | ||
|
||
import semver | ||
|
||
VERSION_REGEX = re.compile( | ||
r"""^[vV]? | ||
(?P<major>0|[1-9]\d*) | ||
(\. | ||
(?P<minor>0|[1-9]\d*) | ||
(\. | ||
(?P<patch>0|[1-9]\d*) | ||
)? | ||
)?$ | ||
""", | ||
re.VERBOSE, | ||
) | ||
|
||
|
||
class InvalidVersion(BaseException): | ||
pass | ||
|
||
|
||
def parse_version(version: str) -> Dict[str, Optional[int]]: | ||
""" | ||
Parses a version string (not necessarily semver) to a dictionary with keys | ||
"major", "minor", and "patch". "minor" and "patch" are possibly None. | ||
|
||
>>> parse_version("0") | ||
{'major': 0, 'minor': None, 'patch': None} | ||
>>> parse_version("1") | ||
{'major': 1, 'minor': None, 'patch': None} | ||
>>> parse_version("1.2") | ||
{'major': 1, 'minor': 2, 'patch': None} | ||
>>> parse_version("1.2.3") | ||
{'major': 1, 'minor': 2, 'patch': 3} | ||
""" | ||
match = VERSION_REGEX.search(version) | ||
if not match: | ||
raise InvalidVersion(f"Could not parse version {version}.") | ||
|
||
return { | ||
key: None if value is None else int(value) | ||
for key, value in match.groupdict().items() | ||
} | ||
|
||
|
||
def vdict_to_vinfo(version_dict: Dict[str, Optional[int]]) -> semver.VersionInfo: | ||
""" | ||
Coerces version dictionary to a semver.VersionInfo object. If minor or patch | ||
numbers are missing, 0 is substituted in their place. | ||
""" | ||
ver = {key: 0 if value is None else value for key, value in version_dict.items()} | ||
return semver.VersionInfo(**ver) | ||
|
||
|
||
def coerce_to_semver(version: str) -> str: | ||
""" | ||
Coerces a version string to a semantic version. | ||
""" | ||
if semver.VersionInfo.is_valid(version): | ||
return version | ||
|
||
parsed_version = parse_version(version) | ||
vinfo = vdict_to_vinfo(parsed_version) | ||
return str(vinfo) | ||
|
||
|
||
def get_caret_ceiling(target: str) -> str: | ||
""" | ||
Accepts a Poetry caret target and returns the exclusive version ceiling. | ||
|
||
Targets that are invalid semver strings (e.g. "1.2", "0") are handled | ||
according to the Poetry caret requirements specification, which is based on | ||
whether the major version is 0: | ||
|
||
- If the major version is 0, the ceiling is determined by bumping the | ||
rightmost specified digit and then coercing it to semver. | ||
Example: 0 => 1.0.0, 0.1 => 0.2.0, 0.1.2 => 0.1.3 | ||
|
||
- If the major version is not 0, the ceiling is determined by | ||
coercing it to semver and then bumping the major version. | ||
Example: 1 => 2.0.0, 1.2 => 2.0.0, 1.2.3 => 2.0.0 | ||
|
||
# Examples from Poetry docs | ||
>>> get_caret_ceiling("0") | ||
'1.0.0' | ||
>>> get_caret_ceiling("0.0") | ||
'0.1.0' | ||
>>> get_caret_ceiling("0.0.3") | ||
'0.0.4' | ||
>>> get_caret_ceiling("0.2.3") | ||
'0.3.0' | ||
>>> get_caret_ceiling("1") | ||
'2.0.0' | ||
>>> get_caret_ceiling("1.2") | ||
'2.0.0' | ||
>>> get_caret_ceiling("1.2.3") | ||
'2.0.0' | ||
""" | ||
if not semver.VersionInfo.is_valid(target): | ||
target_dict = parse_version(target) | ||
|
||
if target_dict["major"] == 0: | ||
if target_dict["minor"] is None: | ||
target_dict["major"] += 1 | ||
elif target_dict["patch"] is None: | ||
target_dict["minor"] += 1 | ||
else: | ||
target_dict["patch"] += 1 | ||
return str(vdict_to_vinfo(target_dict)) | ||
|
||
vdict_to_vinfo(target_dict) | ||
return str(vdict_to_vinfo(target_dict).bump_major()) | ||
|
||
target_vinfo = semver.VersionInfo.parse(target) | ||
|
||
if target_vinfo.major == 0: | ||
if target_vinfo.minor == 0: | ||
return str(target_vinfo.bump_patch()) | ||
else: | ||
return str(target_vinfo.bump_minor()) | ||
else: | ||
return str(target_vinfo.bump_major()) | ||
|
||
|
||
def get_tilde_ceiling(target: str) -> str: | ||
""" | ||
Accepts a Poetry tilde target and returns the exclusive version ceiling. | ||
|
||
# Examples from Poetry docs | ||
>>> get_tilde_ceiling("1") | ||
'2.0.0' | ||
>>> get_tilde_ceiling("1.2") | ||
'1.3.0' | ||
>>> get_tilde_ceiling("1.2.3") | ||
'1.3.0' | ||
""" | ||
target_dict = parse_version(target) | ||
if target_dict["minor"]: | ||
return str(vdict_to_vinfo(target_dict).bump_minor()) | ||
|
||
return str(vdict_to_vinfo(target_dict).bump_major()) | ||
|
||
|
||
def encode_poetry_version(poetry_specifier: str) -> str: | ||
""" | ||
Encodes Poetry version specifier as a Conda version specifier. | ||
|
||
Example: ^1 => >=1.0.0,<2.0.0 | ||
|
||
# should be unchanged | ||
>>> encode_poetry_version("1.*") | ||
'1.*' | ||
>>> encode_poetry_version(">=1,<2") | ||
'>=1,<2' | ||
>>> encode_poetry_version("==1.2.3") | ||
'==1.2.3' | ||
>>> encode_poetry_version("!=1.2.3") | ||
'!=1.2.3' | ||
|
||
# strip spaces | ||
>>> encode_poetry_version(">= 1, < 2") | ||
'>=1,<2' | ||
|
||
# handle exact version specifiers correctly | ||
>>> encode_poetry_version("1.2.3") | ||
'1.2.3' | ||
>>> encode_poetry_version("==1.2.3") | ||
'==1.2.3' | ||
|
||
# handle caret operator correctly | ||
# examples from Poetry docs | ||
>>> encode_poetry_version("^0") | ||
'>=0.0.0,<1.0.0' | ||
>>> encode_poetry_version("^0.0") | ||
'>=0.0.0,<0.1.0' | ||
>>> encode_poetry_version("^0.0.3") | ||
'>=0.0.3,<0.0.4' | ||
>>> encode_poetry_version("^0.2.3") | ||
'>=0.2.3,<0.3.0' | ||
>>> encode_poetry_version("^1") | ||
'>=1.0.0,<2.0.0' | ||
>>> encode_poetry_version("^1.2") | ||
'>=1.2.0,<2.0.0' | ||
>>> encode_poetry_version("^1.2.3") | ||
'>=1.2.3,<2.0.0' | ||
|
||
# handle tilde operator correctly | ||
# examples from Poetry docs | ||
>>> encode_poetry_version("~1") | ||
'>=1.0.0,<2.0.0' | ||
>>> encode_poetry_version("~1.2") | ||
'>=1.2.0,<1.3.0' | ||
>>> encode_poetry_version("~1.2.3") | ||
'>=1.2.3,<1.3.0' | ||
""" | ||
poetry_clauses = poetry_specifier.split(",") | ||
|
||
conda_clauses = [] | ||
for poetry_clause in poetry_clauses: | ||
poetry_clause = poetry_clause.replace(" ", "") | ||
if poetry_clause.startswith("^"): | ||
# handle ^ operator | ||
target = poetry_clause[1:] | ||
floor = coerce_to_semver(target) | ||
ceiling = get_caret_ceiling(target) | ||
conda_clauses.append(">=" + floor) | ||
conda_clauses.append("<" + ceiling) | ||
continue | ||
|
||
if poetry_clause.startswith("~"): | ||
# handle ~ operator | ||
target = poetry_clause[1:] | ||
floor = coerce_to_semver(target) | ||
ceiling = get_tilde_ceiling(target) | ||
conda_clauses.append(">=" + floor) | ||
conda_clauses.append("<" + ceiling) | ||
continue | ||
|
||
# other poetry clauses should be conda-compatible | ||
conda_clauses.append(poetry_clause) | ||
|
||
return ",".join(conda_clauses) |
Oops, something went wrong.
Oops, something went wrong.
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.
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.
can you revert this and add a new step to run the doctests please?
I think it would be better to have the doctests running step separated from the unittests
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.
Cool, thanks so much for the review!
I can certainly rearrange the tests, but could you explain a bit about the current structure so that I can implement it correctly? In particular, should the doctests execute before or after the serial/parallel tests?
I don't understand why the tests are currently split into serial/parallel, and from what I can see they are executing in exactly the same context, so I'm confused. By default, unless there's some good reason, I'd expect the tests to run all together in the same job, so that if some of the tests fail then you see all the failing tests rather than only the failing tests in the particular job.
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 are split into serial and parallel because the serial tests can interfere with other tests and make them fail.