Consider keeping compare module level function

[Askaholic]

I would think that comparing two semver strings is a pretty common use case and this code:

semver.compare("1.2.3", "3.2.1")

is arguably a lot nicer than

semver.VersionInfo.parse("1.2.3").compare("3.2.1")

plus it is probably already present in a large number of projects that use this library, so removing it entirely should be done only if absolutely necessary (which I do not believe that it is).

The fact that VersionInfo.compare accepts the other argument as a string says a lot about the convenience and desirability of working with strings. But it leaves us in this weird situation where VersionInfo.compare takes 2 arguments (self, and other) where self must be a VersionInfo object but other can be one of many types. If we were to write out the type signature we would have something like:

VersionInfo.compare(self: VersionInfo, other: Union[str, dict, list, tuple, VersionInfo])

(Another quirk of this function is that given a tuple as an argument, it will convert to VersionInfo and then immediately back to a tuple again.)

I think as far as API design goes, it doesn't make a lot of sense for the main comparison function to have this type signature. Why is only one argument required to be a parsed VersionInfo object? Why are we doing automatic conversion on one argument but not the other? What if I already have both arguments in tuple form and don't want to perform two useless conversions?

I would suggest having the module level API be a sort of convenience wrapper around the VersionInfo API. I think it is likely that the vast majority of semver data starts out in string form, so there should be a quick and clean API that one can call on this data. I would suggest making semver.compare look like this:

semver.compare(left: Union[str, dict, list, tuple, VersionInfo], right: Union[str, dict, list, tuple, VersionInfo])

And I would probably avoid double conversions on lists and tuples.

[tomschr]

@scls19fr, @ppkt, @gsakkis what's your opinion on this?

[tomschr]

@Askaholic Sorry for the long time, but thanks for your ideas.

I think, it depends on the use case.

If you start with two version strings, then sure, the first variant with semver.compare is indeed shorter. However, you address only one use case. If you already have a version instance, the situation is not so obvious anymore.

For example, you start with a VersionInfo object, do something in between and then compare it afterwards like this:

>>> ver = ...  # ver is an instance of VersionInfo
>>> # do something in between
>>> ver.compare(other)

IMHO, the last line is basically the same then your first line, it's even shorter. It also doesn't matter which types you have in other. This is currently not the case with semver.compare which requires to have two strings.

Regarding this part:

I think as far as API design goes, it doesn't make a lot of sense for the main comparison function to have this type signature.

Not sure why do you think allowing different types is "wrong".
Is it just the implementation or the type signature itself? Or do you think the semver.VersionInfo.compare should allow only one type? Can you elaborate on this?

Why is only one argument required to be a parsed VersionInfo object? Why are we doing automatic conversion on one argument but not the other? What if I already have both arguments in tuple form and don't want to perform two useless conversions?

Normally, for the developer it's a "black box" and how things are converted from one type to another is an implementation detail.

However, that doesn't mean that the implementation couldn't be improved. It was the easiest/simplest for the time being. All other solutions involves some kind of "single dispatch" functionality. But maybe there is better solution which I didn't think of. If you have an idea how this could be improved, I would be more than happy to learn a better solution. Maybe you could add some code snippets of your idea?

I would suggest having the module level API be a sort of convenience wrapper around the VersionInfo API.

Actually, this is already the case. Before version 2.10.0, the functionality was split between module level functions and the VersionInfo class. This is now consolidated and a lot of the implementation was moved into the VersionInfo class. If you look at the existing module level functions, you will see that they all call the VersionInfo API.

See a diff of semver.compare between 2.9.1 and 2.10.0.

I would suggest making semver.compare look like this:
[...]

Theoretically, this is almost the case. See next paragraph:

And I would probably avoid double conversions on lists and tuples.

How?

The current implementation of semver.compare is:

def compare(ver1, ver2):
    """ ... """
    v1 = VersionInfo.parse(ver1)
    return v1.compare(ver2)

When you pass ver1 to VersionInfo.parse, then .parse would need to know the type of ver1. Or we need to extend the initializer of VersionInfo() to allow more. In both cases, the "magic" has to appear inside the class (because module level are only wrappers).

How would you implement these different types and avoid these "double conversions"?

[tomschr]

@tlaferriere would love to hear your opinion on this topic. Should we allow semver.compare and remove the deprecation status? Any arguments for or against it?

[tlaferriere]

I must agree that dealing with version strings is a major use case (not to say the main one) and being able to operate on them directly is extremely convenient.
That said, it is hard to standardize a module level API that can deal with alternative representations of the same idea, and when you lack standardization, maintainability takes a hit. In this case, I think maintainability is more important than convenience and wins out.

This does raise the issue that typing out VersionInfo.parse (that is if you like importing names from other namespaces directly into yours) does eat up almost 20% of your horizontal real estate (of the unofficial 88 character limit that is quite widespread in python). I have found this to be rather inconvenient at times.
This could be easily (from the user pov) avoided by implementing a constructor that can accept a single string like so VersionInfo("1.2.3"). The issue is that it would increase the complexity of the constructor for yet another shortcut.

This gives me an idea: there exist many types of string literals in python (f-strings, r-strings) that use a prefixed letter to distinguish themselves. How hard would it be to add one more (a v-string) that you can activate through an import? (I'm not very familiar with the internals of python, but I suspect this could involve changing the python syntax). But let's forget about the potential hardships of implementing such a feature for a moment.
How awesome would it be if you could write

v"1.2.3".compare(version_string)

And I believe that writing version strings like that is so natural even non-technical people could understand the meaning of this line given the fact that v is used in so many places to indicate a version number.
I envision this feature would basically be syntactic sugar for VersionInfo.parse() and that it would allow you to use all the features of the VersionInfo class from a literal.

In the end, I think that the concision of some obvious and common usages should be addressed and could be improved.
I think we could add an example of an alias in the docs like so:

import semver

v = semver.VersionInfo.parse

v("1.2.3").compare("3.2.1")

[tomschr]

@tlaferriere Thank you very much for your wonderful and logical analysis. 👍 Actually, I had something similar in my mind, but I didn't think that through how you did.

It looks a little bit like Ruby, 😄 but IMHO this would be a good compromise between usability and maintainability.

I'm thinking about a new section like "Migrating from older semver version" which we could integrate into semver 3 doc. There, we could describe this little trick and give recommendations for our developers.

That still leaves one question:

Should we (still) remove semver.compare? If a user really needs it, we could document that in our (yet to be written) new section. It wouldn't be much, but the project would gain more focus.

@Askaholic We can offer now two solutions, what do you think? Would that be an idea for you?

[Askaholic]

I still prefer having a convenient function that I can call directly in the module API rather than needing to write my own wrapper, or renaming one of the module functions just to be able to write readable code. IMO Python is a language of convenience (why else would you choose to write your application in Python, if not because it is easy and a pleasure to write in?) and third party packages should also be written in that spirit.

I understand that there is a tradeoff between convenience and API maintainability, but I would argue that this package isn't at the point where keeping one convenience function is going to put a huge burden on maintenance. The entirety of the code fits in a single 1000 line file (a lot of which is being deprecated and I would agree that it should be deprecated), so I wouldn't exactly say the size of the API is unmaintainable.

[tlaferriere]

Sorry to play the devil's advocate here, but I think it is a good way to really make sure every possible aspect of a decision is thoroughly covered.

Python is a language of convenience

Indeed, in the case of Python prior to 3. From Python 3 onward (I'd say post PEP 484), it is now a major language that is used in production that still values convenience, but only where it doesn't hurt maintainability (type hints aren't exactly convenient to type out, but are extremely valuable for maintainability).
Also, why would you replace the print statement with an actual function call? Well it was in the goal of increasing the coherence of the language (so that it makes more sense).
Last, but not least of python 3 changes: the cmp function is gone, and I think that goes a long way telling us there are better ways comparing things than checking a numeric return value of a function.

Although Python is a convenient language, I think it is attributable to the fact that it is excellently designed around a certain philosophy: The Zen of Python.
In this philosophy, I think the most known tenet is:

There should be one-- and preferably only one --obvious way to do it.

And now we have two ways to compare version strings: the semver.compare function and the VersionInfo.compare method. I'm convinced that this is a clear violation of the previous edict.

Now I understand this is a special case where the current alternative is a character count monstrosity, but as The Zen of Python says:

Special cases aren't special enough to break the rules.

And I tend to think that this is a special case.

...
...
...
...
...

Although practicality beats purity.

@tomschr I've been reading up on mypy recently and I've come across an interesting feature: the @overload decorator. It allows you create multiple precise (and unforgiving) type signatures for a method or function. I think anyone who has once programmed C++ can attest the power of function overloading in a typed language.
Well I think this could be applicable to VersionInfo.__init__ so that we can construct a VersionInfo object from a string, which I believe would be highly practical.

Finally, I've never been a fan of the name VersionInfo. I think the Info part is redundant with the fact that it's a class, kind of like naming a variable like so:

name_string: str = "Thomas"

So there are my two suggestions for this issue:

• Overload the constructor with __init__(self, version: str)
• Rename VersionInfo to Version

With these, you could just write Version("1.2.3").compare("1.2.4") and this should increase the social acceptability of removing semver.compare, while adhering to The Zen of Python One obvious way to do it.
...
...

Although that way may not be obvious at first unless you're Dutch.

[tomschr]

Thank you very much for your detailed answer @tlaferriere! 👍

Sorry to play the devil's advocate here, but I think it is a good way to really make sure every possible aspect of a decision is thoroughly covered.

All good, no need to be sorry. 😄 I appreciate all the input. 👍 I fully agree with you, it's good to hear different aspects and very helpful in the decision process.

Actually, you bring two interesting aspects. Which is funny, because my thoughts went into the same (or almost) same direction. 😆

---

Regarding the @overload decorator
Is that similar to functools.single_dispatch? It looks quite similar, but maybe there are differences. Are you aware of any? Do we only one or do we need both decorators?

It seems the @overload decorator is supported since 3.5 (which would match with our supported Python3 versions). So this is something to be implemented in semver 3.

One question: wouldn't make the overloading of VersionInfo() make the VersionInfo.parse method obsolete?

---

Regarding the rename of VersionInfo to Version.
I was thinking in the same line, but it wasn't high in my priority list. 😉 But as you've mentioned it now, maybe it is a good time to take up that step. We could/should probably add a VersionInfo = Version line somewhere for compatibility reasons (with deprecation notice). In that case, I would prefer a separate issue for that. This issue should be on semver.compare only.

That raises the question if we should prepare this in semver2 already or if this feature should be implemented in semver3 only.

Adding @python-semver/reviewers to notify them.

[tlaferriere]

The @overload decorator is purely for type hints, you can only specify one function body and it has to distinguish the different types using isinstance.
From what I read from functools.single_dispatch it is actually more convenient since you can specify the different function bodies for each signature, but it only distinguishes the first argument's type. OTOH it supports methods only since python 3.8, which rather inconvenient for us.
So I guess for now the only way to clearly signify overloading of __init__ would be to use @overload with type hints.

[tomschr]

Ok, thanks for the hints, @tlaferriere! 👍

As we usually create a VersionInfo object by passing either a string or integers, I would end up with this snippet:

from typing import overload

class VersionInfo:  # or just Version

    @overload
    def __init__(self, major:int, minor:int=0, patch:int=0, prerelease=None, build=None):
        # ...

    @overload
    def __init__(self, version:str):
        # ...

With the above overloaded initializer, we could cover the following use cases:

>>> from semver import VersionInfo as Version
>>> Version(1)
VersionInfo(major=1, minor=0, patch=0, prerelease=None, build=None)
>>> Version(1, patch=2)
VersionInfo(major=1, minor=0, patch=2, prerelease=None, build=None)
>>> Version("1.2.3")
VersionInfo(major=1, minor=2, patch=3, prerelease=None, build=None)
>>> t = (1, 2, 3)
>>> Version(*t)                                             
VersionInfo(major=1, minor=2, patch=3, prerelease=None, build=None)
>>> d = {'major': 3, 'minor': 4, 'patch': 5,  'prerelease': 'pre.2', 'build': 'build.4'}
>>> Version(**d)
VersionInfo(major=3, minor=4, patch=5, prerelease='pre.2', build='build.4')

Do we need anything else? Any other type we need to consider?

With the above changes, the static method VersionInfo.parse would become obsolete.

Would that make sense?

[tlaferriere]

Do we need anything else? Any other type we need to consider?

Well I'm pretty sure you covered them all.

With the above changes, the static method VersionInfo.parse would become obsolete.

Would that make sense?

It makes a lot of sense. I'll try it out in my draft PR #265 and share the learnings here.

[durera]

FWIW, having just updated our code in prep for the deprecation I have to say I agree with the OP that it's a shame we can't compare plain version strings with the same ease anymore...

from:

return semver.compare(version1, version2) >= 0

to:

theVersion = semver.VersionInfo.parse(version1)
compareToVersion = semver.VersionInfo.parse(version2)
return theVersion.compare(compareToVersion) >= 0

or, if you like long lines:

semver.VersionInfo.parse(version1).compare(semver.VersionInfo.parse(version2)) >= 0

Maybe there's a cleaner way in the new world to get the comparison done when starting out with two strings though? Either way this is a really useful package, thanks to all involved 👍

[tomschr]

@durera sorry for the delay. Yes, it looks a bit verbose. Some time ago, I've added in #303 an idea which is related to this issue:

• rename VersionInfo to Version.
• get rid of this .parse() method.
• make the __init__ ("constructor") more flexible and allow different calls, like with integers or string

With these changes it could be simplified like this:

# version1 and version2 can be str type
semver.Version(version1).compare(version2)

what do you think?

[tomschr]

I just released 3.0.0-rc.1, but this issue wasn't included. Haven't forgot about it, still thinking which was the best way to solve it.

There are a lot of good arguments from both sides. As we don't have a uniform initializer yet, it seems we still need Version.parse. If I get a better idea how we could improve the Version.__init__, I'm inclined to include semver.compare in 3.0.0-rc2 for the time being.

This would give us some benefits:

• We can release 3.0.0 with semver.compare, but we could still mark it as "pending for deprecation".
• We could work on an overloaded initializer to allow multiple types; maybe we need to release major 4 of semver if we break the API.
• Although it's maybe not ideal to have two solutions, we could slowly work towards a better solution. To make semver.compare obsolete or to improve the others to make them more readable.
• Gives developers more time to improve their code (as 3.0.0 already has some big changes).
• Perhaps we could even cover this in the documentation, if someone is really in need of this.

So maybe that gives us some more time to let it sink and think about it. People can still use semver.compare for the time being. Maybe the situation will change in semver 4, I don't know yet.

Nevertheless, I reserve the right to remove it in version 4 if it turns out that this decision was bad or we have a better way until then. 😉

I plan to integrate it into 3.0.0-rc.2.

[llucax]

Hi @tomschr!

I just arrived to this issue while trying to use semver.compare() which is part of the examples in both the README and documentation. When I use it, I notice it is deprecated (or pending deprecation, but still marked as deprecated, so shown as such in my editor) and it points to this issue, which is closed and doesn't have a good high-level summary explaining users what's the status. Also there is a section in the docs explaining how to migrate to 3.0 and in particular a subsection about how to replace deprecated functions with absolutely no mention to compare().

This all makes a very confusing first impression of the library.

The situation could be improved by:

• Using the non-deprecated form in the readme and documentation.
• Maybe opening the issue while the deprecation is not resolved
• Adding a summary of the situation to the top of the issue, and/or...
• Adding a summary of the situation and migration steps in the documentation section about replacing deprecated function.
• Adding an upgrade path to the deprecation message.

Thanks for the great library and I hope this could help other users to have a smoother start with the library :)

[llucax]

As an extra clarification, as I see you are not using the upcoming standard deprecated decorator, I saw the deprecation because I'm using pyright, which already supports it, so @deprecated as a special handling. Here is how it is seen in my editor:

So maybe not that many users are affected by this, but it is something you might consider for the future.

[tomschr]

Thanks @llucax for your feedback. Much appreciated that! 👍

The semver.compare() function was a kind of exception to the deprecation. I've tried to consolidate all the module level functions in favor to the semver.Version class based functions. This semver.compare() function was somewhat controversial as we still don't have yet a good, short replacement.

I usually try hard to keep the docs up-to-date. However, in this case, due to the controversial nature, I was cautious to mention this. As a writer myself, I usually add something to the docs when it's stable, proven, and correct. As this was not the case with semver.compare() so I didn't mention it. Maybe I should have.

At least you stumbled upon this, so the deprecation did its job. 😉

Thanks for mentioning PEP 707. At the moment, this is still in draft state, so I'm not sure how this could help. But I will keep in mind.

If you think you could improve the situation, feel free to open a pull request or discuss it here. 🙂

Thanks again!