Version tuple parsing #273
Conversation
…d versions Signed-off-by: Sergey Kastryulin <[email protected]>
Signed-off-by: Sergey Kastryulin <[email protected]>
Signed-off-by: Sergey Kastryulin <[email protected]>
Signed-off-by: Sergey Kastryulin <[email protected]>
Signed-off-by: Sergey Kastryulin <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #273 +/- ##
==========================================
- Coverage 93.28% 93.25% -0.04%
==========================================
Files 33 33
Lines 2264 2283 +19
==========================================
+ Hits 2112 2129 +17
- Misses 152 154 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Ready for review. |
| if isinstance(version, bytes): | ||
| version = version.decode("UTF-8") |
There was a problem hiding this comment.
What is the use case for that? Have you ever met library whose version is encoded in raw bytes instead of string?
IMO this is redundant, but if there are such cases with raw bytes — let's keep it.
There was a problem hiding this comment.
Byte string is an option. Whether this option is used depends of a library's authors.
This behaviour is processed by the python-semver library, which is the reason why I decided to include it as well. I do not consider an additional sanity check to be redundant.
| match = _REGEX.match(version) | ||
| if match is None: | ||
| warnings.warn(f"{version} is not a valid SemVer string") | ||
| return tuple() |
There was a problem hiding this comment.
Why return empty tuple instead of explicit None?
There was a problem hiding this comment.
Practical reason: mypy won't let you do that because Optional[Tuple[int, ...]] is later used in the >= operation. The linter cannot see that the check that verifies that the value is not None is done and reports that the >= operator for (possibly) None value makes no sense — more on that in this action report.
Conceptual reason: functions that return different types and number of arguments depending on internal logic are not good because they force independent (outside) code to have a specific behavior to encounter for that. Yes, I know that in the case of an empty tuple, we add a specific check anyway, but if we didn't, it at least wouldn't crash.
Personally, I don't see the conceptual reason to be too big of a problem, but then the practical reason comes into play 😄
| _REGEX = re.compile( | ||
| r""" | ||
| ^ | ||
| (?P<major>0|[1-9]\d*) | ||
| \. | ||
| (?P<minor>0|[1-9]\d*) | ||
| \. | ||
| (?P<patch>0|[1-9]\d*) | ||
| (?:-(?P<prerelease> | ||
| (?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*) | ||
| (?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))* | ||
| ))? | ||
| (?:\+(?P<build> | ||
| [0-9a-zA-Z-]+ | ||
| (?:\.[0-9a-zA-Z-]+)* | ||
| ))? | ||
| $ | ||
| """, | ||
| re.VERBOSE, | ||
| ) |
Signed-off-by: Sergey Kastryulin <[email protected]>
Signed-off-by: Sergey Kastryulin <[email protected]>
Signed-off-by: Sergey Kastryulin <[email protected]>
|
Kudos, SonarCloud Quality Gate passed!
|
|
@denproc done, please confirm |
Closes #272
Proposed Changes
gudhi), the execution does not fail with an exception, warning is provided instead.