Skip to content
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.

Sign up for GitHub

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

Jump to bottom

PEP440 Version output changes between test and update #221

Open
wordsworthc opened this issue Sep 22, 2023 · 3 comments · May be fixed by #225
Open

PEP440 Version output changes between test and update #221

wordsworthc opened this issue Sep 22, 2023 · 3 comments · May be fixed by #225

Comments

@wordsworthc
Copy link

I noticed that running bumpver test ... produces a different pep440_version output than when running bumpver update ....

Consider the following version patterns: vMAJOR.MINOR.PATCH[-PYTAGNUM], and vMAJOR.MINOR.PATCH[+localINC0], each has a very different meaning according to pep 440.

In all of my examples below I use these file patterns in my pyproject.toml:

[tool.bumpver.file_patterns]
"pyproject.toml" = [
    'version = "{pep440_version}"',
    'current_version = "{version}"',
]

vMAJOR.MINOR.PATCH[-PYTAGNUM]

Testing the above pattern with bumpver I get:

(venv) PS D:\dev\my-project> bumpver test "v1.0.0" "vMAJOR.MINOR.PATCH[-PYTAGNUM]" --tag-num --tag post
New Version: v1.0.0-post0
PEP440     : 1.0.0.post0

As expected, the PEP440 output is a pep440-compliant post-version: 1.0.0.post0. However, when I try the update command:

(venv) PS D:\dev\my-project> bumpver update --no-fetch --ignore-vcs-tag --tag-num --tag post --dry
INFO    - Old Version: v1.0.0
INFO    - New Version: v1.0.0-post0
--- pyproject.toml
+++ pyproject.toml
@@ -4,7 +4,7 @@

 [project]
 name = "my-project"
-version = "1.0.0"
+version = "1.0.0post0"
 dynamic = ["readme", "dependencies"]


@@ -32,7 +32,7 @@


 [tool.bumpver]
-current_version = "v1.0.0"
+current_version = "v1.0.0-post0"
 version_pattern = "vMAJOR.MINOR.PATCH[-PYTAGNUM]"
 commit = "True"
 tag = "True"

Notably, the pep440_version output is different (Although it is still pep440-compliant), resulting in the version 1.0.0post0.

vMAJOR.MINOR.PATCH[+localINC0]

Testing this pattern with bumpver I get:

(venv) PS D:\dev\my-project> bumpver test "v1.0.0" "vMAJOR.MINOR.PATCH[+localINC0]"
New Version: v1.0.0+local1
PEP440     : 1.0.0+local1

This time, as expected, the PEP440 output is a pep440-compliant local version. Now for bumpver update ...:

(venv) PS D:\dev\my-project> bumpver update --no-fetch --ignore-vcs-tag --dry
INFO    - Old Version: v1.0.0
INFO    - New Version: v1.0.0+local1
--- pyproject.toml
+++ pyproject.toml
@@ -4,7 +4,7 @@

 [project]
 name = "my-project"
-version = "1.0.0"
+version = "1.0.0local1"
 dynamic = ["readme", "dependencies"]


@@ -32,7 +32,7 @@


 [tool.bumpver]
-current_version = "v1.0.0"
+current_version = "v1.0.0+local1"
 version_pattern = "vMAJOR.MINOR.PATCH[+localINC0]"
 commit = "True"
 tag = "True"

Herein lies the true nature of this issue - even though I have specifically configured bumpver to provide local versions (and used bumpver 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:

(venv) PS D:\dev\my-project> pip freeze
bumpver==2023.1126
click==8.1.7
colorama==0.4.6
lexid==2021.1006
looseversion==1.3.0
toml==0.10.2
@mbarkhau
Copy link
Owner

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.

@wordsworthc
Copy link
Author

wordsworthc commented Oct 30, 2023 •
edited
Loading

@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 test and update (using --dry). I refactored the implementation so that test uses the same code to translate to pep440 as update.

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 {pep440_version} replacement pattern does not seem to work in these cases either. For now I have worked around this by falling back to the old pep440 handling in this specific case, which at least means those v1 patterns can be used without the {pep440_version} replacement pattern.

Ultimately I chose not to change the handling of update, because I expect that current users would be relying more heavily on those results, and changing those results is most likely to upset a few people. Please take a look and let me know your thoughts. I am interested to get a few opinions on this.

See changes: #225

@wordsworthc wordsworthc linked a pull request Oct 30, 2023 that will close this issue
Open
@mbarkhau
Copy link
Owner

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 test/test_cli.py which appears to be unrelated to this issue. If you want to create a separate PR just for your work on refactoring of the tests, or reduce the existing one to just that, I'd be happy to merge those changes.

Secondly, the output of bumpver test produces what should be used as the correct pep440 version. The critical function to look at is v2patterns.py@_convert_to_pep440. Incidentally this function has a preamble about its limitations which we are now running into:

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 _convert_to_pep440. I don't think there is any other option.

Aside: I considered if there is a larger scale refactoring that might allow us to avoid using the _convert_to_pep440 function. For example, we might not parse the files using regular expressions, but rather generate a search string using the current version and a replacement string with the incremented version. This would have the disadvantage, that we could only replace versions that are up-to-date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Pep440 version consistency wordsworthc/bumpver
2 participants
@mbarkhau @wordsworthc