-
Notifications
You must be signed in to change notification settings - Fork 36
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
PEP440 Version output changes between test and update #221
Comments
I have a sneaking suspicion this has something to do with character escaping and regular expressions. I won't have time to look at this closer for at least a few weeks. If you can debug this and create a PR with some tests, i'd be happy to merge it. |
@mbarkhau I have only recently had time to come back to this myself. I have refactored some of the cli unit tests to check results from However, I ran into a problem with this approach where some (most?) possible v1 patterns don't have a useful mapping to a pep440-compliant format, resulting in no handling whatsoever for such patterns. I note that using the Ultimately I chose not to change the handling of See changes: #225 |
I spend some time looking at this today. Thanks for your work so far. Firstly, regarding the PR. I'm not inclined to accept it atm. primarily as I don't see it addressing the issue. On its other merits, I'd be inclined to accept it if it were just the refactoring of Secondly, the output of def _convert_to_pep440(version_pattern: str) -> str:
# NOTE (mb 2020-09-20): This does not support some
# corner cases as specified in PEP440, in particular
# related to post and dev releases. We can agree to not break any existing use cases, but there is perhaps a way we can move forward with a bugfix that people can opt into using a config parameter. Of course it makes little sense to look at maintaining backward compatibility, if we haven't even implemented any change. What I think we should do is fix the corner cases of Aside: I considered if there is a larger scale refactoring that might allow us to avoid using the |
I noticed that running
bumpver test ...
produces a differentpep440_version
output than when runningbumpver update ...
.Consider the following version patterns:
vMAJOR.MINOR.PATCH[-PYTAGNUM]
, andvMAJOR.MINOR.PATCH[+localINC0]
, each has a very different meaning according to pep 440.vMAJOR.MINOR.PATCH[-PYTAGNUM]
Testing the above pattern with bumpver I get:
As expected, the PEP440 output is a pep440-compliant post-version:
1.0.0.post0
. However, when I try the update command:Notably, the
pep440_version
output is different (Although it is still pep440-compliant), resulting in the version1.0.0post0
.vMAJOR.MINOR.PATCH[+localINC0]
Testing this pattern with bumpver I get:
This time, as expected, the PEP440 output is a pep440-compliant local version. Now for
bumpver update ...
:Herein lies the true nature of this issue - even though I have specifically configured
bumpver
to provide local versions (and usedbumpver test ...
to make sure it would work), the end result is not pep440-compliant.Expected Behaviour
Although I am not too heavily invested in the final resolution, it is my opinion that the current
bumpver test
output best matches expectations in the examples provided.Environment
Tested in Python 3.10.6 and Python 3.11.3. Both environments use the same package versions:
The text was updated successfully, but these errors were encountered: