bump_prerelease and bump_build return smaller version

The current behavior of bump_prerelease and bump_build seems wrong, or at least unexpected:

In [64]: v1 = semver.parse_version_info('0.1.4')                                                                                              

In [65]: v1 < v1.bump_major()  # good
Out[65]: True

In [66]: v1 < v1.bump_minor()  # good                                                                                                               
Out[66]: True

In [67]: v1 < v1.bump_patch()  # good                                                                                                    
Out[67]: True

In [68]: v1 < v1.bump_prerelease()   # not good                                                                                                       
Out[68]: False

In [69]: v1 < v1.bump_build()  # not good                                                                                                    
Out[69]: False

Is this deliberate? I would expect them to return what v1.bump_patch().bump_prerelease() and v1.bump_patch().bump_build() return now.

@gsakkis

@gsakkis Thanks for your report. George!

Actually I wasn't sure, so I've looked into the Semver spec. Let's start first with the build part.

Under paragraph 10 you can see:

[...] Build metadata MUST be ignored when determining version precedence.

and in paragraph 11 (emphasize by me):

Precedence refers to how versions are compared to each other when ordered. Precedence MUST be calculated by separating the version into major, minor, patch and pre-release identifiers in that order (Build metadata does not figure into precedence). [...]

To my understanding, if you raise only the build part, it doesn't matter. So when you compare 0.1.4 with 0.1.4+build.1 you still compare the second with 0.1.4. So basically, you say 0.1.4 < 0.1.4. It would be the same than comparing 1 < 1 which Python considers as False.

So this comparison would be still correct:

>>> v1 < v1.bump_build()
False

Regarding the prerelease part, paragraph 11 gives you this example:

1.0.0-rc.1 < 1.0.0

Applied to your example, it would be the same as:

>>> v1.bump_prerelease() < v1  # 0.1.4-rc.1 < 0.1.4
True

which would be the same than:

>>> v1 < v1.bump_prerelease()  # 0.1.4 < 0.1.4-rc.1
False

and this would be correct. Do you agree? I would mark it as WONTFIX, if my findings are correct.

However, if you find this surprising, others may find that as well. What do you think of adding this to the documentation?

@python-semver/reviewers can you confirm?

@tomschr

@tomschr thanks for the detailed response

>>> v1 < v1.bump_prerelease()  # 0.1.4 < 0.1.4-rc.1
False
and this would be correct. Do you agree?

I didn't/don't dispute that 0.1.4 is indeed not less than 0.1.4-rc.1; the comparisons work perfectly fine as is. I argue that semantically "bump" should by definition return something higher:

bump noun [C] (INCREASE)
 
an increase in something:
There had been a bump in the number of students with learning disorders.

So v1.bump_prerelease() should return the smallest pre-release version that is higher than v1, which in this example is 0.1.5-rc.1.

@gsakkis

@gsakkis Oh I see! 💡 I agree, from a semantical point of view I concur with you. The name is ambiguous.

Unfortunately, this would break the API contract. IF we really want to change that, this could only be done in a major release. However, I'm not sure if we should do that... 🤔 I think, a lot of people would be surprised. Additionally, bump_build() would behave differently than the other bump methods.

Well, I see the following options:

Change the API in major release 3 (so return 0.1.5-rc.1 instead of 0.1.4-rc.1).
Pros: would be in sync with its name.
Cons: would break API from major version 2.x.y.
Document the ambiguos behaviour in the docstring for bump_build.
Pros: when documented in the docstring, developers may be aware from their IDE (popup menu?)
Cons: the ambiguity is still there
Offer an additional, optional argument (for example, strict, or better raiseprerel?) which could switch between the "old" and "new" behaviour.
Pros: both behaviours can be used
Cons: could confuse developers (do I need it?); what should we set as default?
Create an additional method
Pros: API of bump_build would be unmodified; both could co-exist together
Cons: seems awkward to use just an additional function for the build part; maybe not obvious or confusing to developers (which one do I need?)

Maybe you have another idea which isn't listed above?

Have you looked at other libraries? How do they deal with this?

@tomschr

@tomschr The only other library I checked is python-semanticversion which doesn't currently provide a method to bump prerelease (or build); I opened an issue for it.

Among the options you listed I'd be in favor of 4, though not specifically for the build part but all of them. A possible signature could be:

def next_version(version, part, *, prerelease_token="rc", build_token="build"):

where part can be one of {"major", "minor", "patch", "prerelease", "build }.

By the way, the 0.1.4→0.1.5-rc.1 transition isn't the only problematic one:

In [9]: semver.bump_patch("1.2.4-rc3")                                                                                                                                    
Out[9]: '1.2.5'

I would expect that the next patch version of 1.2.4-rc3 is 1.2.4, not 1.2.5.

So currently there are two special cases, without even considering the build part.

@gsakkis

@gsakkis Thank you George for your answers and ideas! 👍 Much appreciated!

I was more looking for other, non-python libraries and how they deal with the problem. So there is probably a semver library in Java, Ruby, Go, PHP, etc. 😉 If you like, could you have a look at these?

Your suggestion with next_version() looks interesting, I like it.

I guess, we need next_version() as a normal function semver.next_version and as a method for the VersionInfo class like semver.VersionInfo.next_version() class, right? Of course with a slightly different argument list.

So just to be on the same page, a session would look like this:

import semver
>>> v1 = semver.parse_version_info('0.1.4')
>>> str(v1.next_version("major"))
'1.0.0'
>>> str(v1.next_version("build"))
'0.1.4+build.1'
>>> str(v1.next_version("prerelease"))
'0.1.5-rc.1'
>>> v2 = semver.parse_version_info('1.2.4-rc3')
>>> str(v2.next_version("patch")
'1.2.4'

I hope I got it right.

@tomschr

@tomschr yes we're on the same page 👍

How about prereleases of minor and major releases? They probably should be consistent with patch:

>>> v3 = semver.parse_version_info('1.3.0-rc3')
>>> str(v3.next_version("minor"))
'1.3.0'

>>> v4 = semver.parse_version_info('2.0.0-rc4')
>>> str(v4.next_version("major"))
'2.0.0'

However I'm unsure what the following should return:

>>> str(v3.next_version("patch"))
>>> str(v4.next_version("patch"))
>>> str(v4.next_version("minor"))

@gsakkis

@gsakkis Great! 👍 Thanks for the reply. I think, it's starting to take shape. 😉

If I'm not mistaken, the first example from your last comment could contain a logical error. Not entirely sure about this, but it looks wrong to me. IMHO it should raise the minor part:

>>> v3 = semver.parse_version_info('1.3.0-rc3')
>>> str(v3.next_version("minor"))
'1.4.0'

this would allow to do this:

>>> str(v3.next_version("patch"))
'1.3.0'

that would be consistent with my last comment #221 (comment).

Does that make sense?

@tomschr

@tomschr since 1.3.0-rc* is strictly before 1.3.0, I think about it as an approximation of 1.2.9999...; the next minor version of the latter is 1.3.0. Similarly, '2.0.0-rc*' is like 1.9999...9999...., whose next major version is 2.0.0.

If we accept this formulation, you can see why (for example) v3.next_version("patch") is problematic: the next patch version of 1.2.X is 1.2.(X+1), which is infinite/undefined if X is infinite/undefined.

Apart from what we want to return, I'm thinking about your signature:

def next_version(version, part, *, prerelease_token="rc", build_token="build"):

Especially the part argument. Currently it's a string. And if you pass a string you can pass all sorts of typos. 😉 Additionally, if the Semver spec would ever rename these parts (probably very unlikely, but you never know...), this relationship wouldn't work anymore. (Probably we need to overhaul the entire library, but...)

So I'm looking for a more "general" solution, prefereably something that isn't attached to these strings. More of a constant. An Enum? Different variants like next_major_version() etc.?

Maybe I'm overthinking that. 😄

@ppkt

since 1.3.0-rc* is strictly before 1.3.0, I think about it as an approximation of 1.2.9999...; the next minor version of the latter is 1.3.0. Similarly, '2.0.0-rc*' is like 1.9999...9999...., whose next major version is 2.0.0.

I see. Yes, that makes sense.

If we accept this formulation, you can see why (for example) v3.next_version("patch") is problematic: the next patch version of 1.2.X is 1.2.(X+1), which is infinite/undefined if X is infinite/undefined.

Hmn.... 🤔 ... I need a bit more time to think and reflect on this. I will do some research.

@scls19fr @ppkt any hints, ideas, opinions?

First implementation of python-semver#221

@gsakkis

@gsakkis I've added a first implementation. It's a draft and contains probably errors. Please have a look if this would match your idea. It contains also some test cases.

I slightly deviated from your original function signature. You've suggested:

def next_version(version, part, *, prerelease_token="rc", build_token="build"):

I've changed it in this implementation to:

def next_version(version, part, prerelease_token="rc"):

For this implementation I didn't allow "build" as a valid part. It's still a big foggy. Maybe we shouldn't allow it. The build part and the build token seems to be a bit unnecessary to me. Additionally, it seems the Semver spec doesn't take it into account too. Is there a use case? I might be wrong. We could add it later, if we decide it could be useful or necessary (consistency).

Furthermore, the "*" wasn't compatible with Python 2. As we haven't moved to the Python 3 exclusively, I had to remove that too. We can add it once we create the semver3 release.

@tomschr

@tomschr thanks for taking the time to come up with a draft, I'll take a look at it! The signature looks good, I agree that build is less important and clear; we can always extend it later if necessary.

Note to myself:

Extend docstring of bump_* versions with a short hint "See also next_version".

Navigation Menu

Repository navigation

bump_prerelease and bump_build return smaller version #221

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

bump_prerelease and bump_build return smaller version #221

Description

Activity

tomschr commented on Feb 20, 2020

gsakkis commented on Feb 21, 2020

tomschr commented on Feb 21, 2020

gsakkis commented on Feb 21, 2020

tomschr commented on Feb 21, 2020

gsakkis commented on Feb 21, 2020

tomschr commented on Feb 21, 2020

gsakkis commented on Feb 21, 2020

tomschr commented on Feb 21, 2020

tomschr commented on Feb 21, 2020

tomschr commented on Feb 23, 2020

gsakkis commented on Feb 24, 2020

tomschr commented on Apr 20, 2020

Metadata

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Issue actions