Fix #284: implement "is compatible with" method

[tomschr]

This is a first implementation of Version.is_compatible.

The algorithm checks:

• if the two majors are different, it's incompatible
• if the two majors are equal, but the minor of the callee is lower than the caller, it's incompatible
• if both prereleases are present and different, it's incompatible
• otherwise it's compatible

@rafalkrupinski /@sbrudenell / @Lexicality maybe you want to try/review it? 🙂

TODOs

• Improve changelog
• Improve docstring with example(s)
• Add missing documentation
• Review code
• Clarify naming

[tomschr]

Thanks @Lexicality for your review! Much appreciated! 👍

[Lexicality]

Probably worth mentioning in the docstring that this comparison ignores patch versions and giving a couple of examples?

[tomschr]

Probably worth mentioning in the docstring that this comparison ignores patch versions and giving a couple of examples?

Yes, also something for the documentation. On my todo list. 🙂

[tomschr]

Hi @Lexicality (and to whom who are interested):
I worked on the issue and updated this PR. Would you mind having a look again? Especially on the test cases and the implementation.

Thank you very much!

[rafalkrupinski]

@tomschr great work!

Tests comparing a release with a pre-release and major-0 versions (initial development) are missing;
these should all return false unless versions are equal.

[tomschr]

great work!

Thanks. 😊

Tests comparing a release with a pre-release and major-0 versions (initial development) are missing; these should all return false unless versions are equal.

@rafalkrupinski Ahh yes, right. I'm not completely sure if I understood your sentence. Do you mean something like this:

>>> ver = Version(0, 0, 0, "rc.1")
>>> ver.is_compatible(Version(...))  # ???

[rafalkrupinski]

Ah, poor grammar, sorry.

• all major-0 versions should be incompatible with anything but itself, not even when only the patch patch is different (https://semver.org/#spec-item-4)

  Version(0,1,0).is_compatible(Version(0,1,1)) # returns False
  Version(0,1,0).is_compatible(Version(0,1,0)) # True - only identical versions are compatible

• tests checking compatibility between release and pre-release versions (https://semver.org/#spec-item-9)

  Version(1,0,0).is_compatible(Version(1,0,0,'rc1.')) # returns False

[tomschr]

@rafalkrupinski Thanks Raphael for your help. 👍 I've amended your examples in test_semver.py. There is one question left.

If I read item 9 from the Semver spec correct, the other way around, it should be True, right?

Version(1,0,0, 'rc1.').is_compatible(Version(1,0,0))
# returns True

Because, 1.0.0-rc1. < 1.0.0. Unfortunately, our implementation doesn't take this into account. You can see commit 37733f3 fails. Do I miss something?

[rafalkrupinski]

Because, 1.0.0-rc1. < 1.0.0. Unfortunately, our implementation doesn't take this into account. Do I miss something?

I think you're referring to this:

Pre-release versions have a lower precedence than the associated normal version.

That's just ordering, not compatibility.
And BTW, clearly pre-release is always < release, but how would you compare two pre-releases? They can be anything, not just rc, dev or alpha.

A pre-release version indicates that the version is unstable and might not satisfy the intended compatibility requirements as denoted by its associated normal version.

That's False to me.

[tomschr]

A pre-release version indicates that the version is unstable and might not satisfy the intended compatibility requirements as denoted by its associated normal version.

That's False to me.

Ok, that's it! Thanks. I've corrected the tests now. Would you go through it and have a final look?

[tomschr]

Oh, one thought that just came into my mind: do we want to make the is_compatible visible also the our CLI version?

[rafalkrupinski]

@tomschr
I've edited my previous answer to mention ordering.
Also:
https://github.com/python-semver/python-semver/pull/368/files#diff-d6e8c013b9e7be774edf21bed5411e65f2a2bf6b43110b67e6c4dccdcab23791R683
https://github.com/python-semver/python-semver/pull/368/files#diff-d6e8c013b9e7be774edf21bed5411e65f2a2bf6b43110b67e6c4dccdcab23791R689

[tomschr]

I've edited my previous answer to mention ordering.

Thanks

And BTW, clearly pre-release is always < release

Yes, that was also my take on this. 😄 Probably I was confused between ordering and compatibility.

You've mentioned these two lines:

• if not isinstance(new, type(self)):
  Not sure what's wrong with that. We check first, if we have compatible Version classes. If not, we don't even try to convert them.

• if (0 == self.major == new.major) and (self[:3] != new[:3]):
  Also not sure what's wrong either. We need a special if-clause to catch major-0 releases.

The tests are okay now. Maybe could you elaborate on the two lines, please? Many thanks!

[rafalkrupinski]

I don't know how to link comments on code...

Why checking against type(self)?
Are you're expecting users to subclass Version? But then user couldn't compare custom Version with this one

class CustomVersion(Version):
    ...

CustomVersion(1,0,0).is_compatible(Version(1,0,0)) # raises TypeError

Is that the intention?

Doesn't [:3] make 0.1.0 and 0.1.0-rc1 return True?

[tomschr]

Why checking against type(self)?
Are you're expecting users to subclass Version? But then user couldn't compare custom Version with this one

class CustomVersion(Version):
    ...

CustomVersion(1,0,0).is_compatible(Version(1,0,0)) # raises TypeError

Is that the intention?

Well, not really. My intention was to check against incompatible types, as "anything that is not a Version" is incompatible. Of course, a derived class from Version should be allowed.

Ahh, you are right, this expression:

if not isinstance(new, type(self)):

should be:

if not isinstance(new, Version):

Not sure why I came up with this nonsense. 😄 I will amend the tests.

Doesn't [:3] make 0.1.0 and 0.1.0-rc1 return True?

Ahh, right. Good catch! It gives back major, minor, and patch. Not the pre-release. Should be self[:4] != new[:4].

Both parts are now added (the first part contains also an additional test case).

[tomschr]

@Lexicality, @tlaferriere anything that should be changed or improved from your side? Thanks!

[tomschr]

@Lexicality, @tlaferriere, @sbrudenell, @rafalkrupinski Thanks for all your help. Could you have a final look at this PR?

The changes what I did was mostly about documentation: document the deprecation of Version.isvalid() and rephrased some text. The code is not changed from last time.

Please let me know if you are okay with that. Don't stress yourself, it's fine after the public holidays. 😉 If you don't object I would merge it some time next week.

Many thanks!

[rafalkrupinski]

https://github.com/python-semver/python-semver/pull/368/files#diff-20b056eb54729a600ccb91efc5249db4fb816c4d964bdbc7963641694ee0439aR7

Not sure about the wording of this section.

• All conditions must be met, but bullet 2 is repeated as part of bullet 3.
• bullet 3 - minor is lower or equal
• bullet 4 - both are present and equal or both are absent.

Alternatively:

a.is_compatible(b) is True when either is true:

• both versions are identical, or
• both are releases, their majors are equal and higher than 0 and minor versions, or
• both are releases, their majors are equal and higher than 0 and b's minor version is higher then a's

---

I'm missing instruction on how to use it in real life.

E.g.:

• when package is reading a versioned JSON file, should it check package_version.is_compatible(file_version) or the other way around?
• when an executable works with library/API in version X, would it also work with version Y? Should it check X.is_compatible(Y)?

Basically document the caller/callee description that we ditched earlier.
(We want caller.is_compatible(callee) and in the examples file_version.is_compatible(consumer_version) and yes)

And BTW, I think this best describes the method and it makes its behaviour clear:

a.is_compatible(b) checks if the change from version "a" to version "b" is (declared to be) compatible according to SemVer rules.

---

Code examples

• I find it harder to read without a=Version... for every example
• it would be nice to include reverse call b.is_compatible(a); extra points if you know why I skipped it from the last example :P
• missing example with different minor, resulting with True
• two examples with pre-release are essentially the same
• is that dot in 'rc1.' on purpose?

    # Two different majors:
    >>> a = Version(1, 1, 1) 
    >>> b = Version(2, 0, 0) 
    >>> a.is_compatible(b)
    False
    >>> b.is_compatible(a)
    False

    # Two different minors:
    >>> a = Version(1, 1, 0) 
    >>> b = Version(1, 0, 0)
    >>> a.is_compatible(b)
    False
    >>> b.is_compatible(a)
    True

    # The same two majors and minors:
    >>> a = Version(1, 1, 1) 
    >>> b = Version(1, 1, 0) 
    >>> a.is_compatible(b)
    True
    >>> b.is_compatible(a)
    True

    # Release and pre-release:
    >>> a = Version(1, 1, 1)
    >>> b = Version(1,0,0,'rc1')
    >>> a.is_compatible(b)
    False
    >>> b.is_compatible(a)
    False

    # Different pre-releases:
    >>> a = Version(1, 0, 0, 'rc1')
    >>> b = Version(1, 0, 0, 'rc2')
    >>> a.is_compatible(b)
    False
    >>> b.is_compatible(a)
    False

    # Identical pre-releases
    >>> a = Version(1, 0, 0,'rc1')
    >>> b = Version(1, 0, 0,'rc1')
    >>> a.is_compatible(b)
    True

[tomschr]

@rafalkrupinski Thanks a lot Raphael. This is gold! 🥇

You are right with the examples. I used your code and added it there. 👍 Regarding the real-life examples, I agree. I'm still trying to find good a explanation and wording. I will amend it later.

I have some questions/comments regarding your description of the a.is_compatible(b) is True:

• Using the expression a.is_compatible(b) is True is very useful. 👍 I like that!
• "both versions are identical".
  You mean both versions are equal, right? 😉 Sorry, but I think we should be consistent here as "identical" smells like we are comparing identity with the is operator instead of equality with ==. You probably meant the latter.
• What do you mean by "both are releases"?
  I suppose something like both versions contain prereleases which are equal?
• "their majors are equal and higher than 0 and minor versions"
  That sounds a bit strange to me. If I got it right, you mean that the minors of both versions are also equal and higher than 0. Practically the same conditions than the major's, right?
  I would recommend to dissect it into different main clauses to make it easier to read.
• The order
  In your second and third item, you start with "both are releases". Hmn, that means you're putting the cart before the horse? 😉
  Usually we interpret a version from left to right (first major, then minor, then patch, etc.). As such, I would mention the major condition(s) first before explaining the other parts.

When I apply these changes from above, this would lead to:

---

The expression a.is_compatible(b) is True if one of the following statements is true:

• both versions are equal, or
• both majors are equal and higher than 0. The same applies for both minor parts. Both pre-releases are equal, or
• both majors are equal and higher than 0. The minor of b's minor version is higher then a's. Both pre-releases are equal.

---

I know, it's longer. However, I think, it's clearer.

Many thanks and much appreciated your feedback! 👍

[rafalkrupinski]

1. You should see people writing poems in Perl 4 ;)
2. Of course, equal. I used identical because definitions of equality can vary (but I guess it's well defined in this case).
3. By release I mean not a pre-release. Pre-releases need to be equal to be compatible so they fall in the rule 1. The two other rules apply only to release versions. Same with major-zero

Practically the same conditions than the major's, right?

Minor parts don't have to be higher than zero to be compatible.

5. Yeah it is clearer when it's less condensed. SemVer explains it in 9 points :)
   (Although, they also define some ordering rules)

• they are equal, or
• all of:
  • their major parts are higher than 0 and equal, and
  • neither of them is a pre-release, and
  • b's minor is greater than or equal a's

[tomschr]

1. You should see people writing poems in Perl 4 ;)

😆 Luckily, we don't have to follow Perl. 😉

3. By release I mean not a pre-release. Pre-releases need to be equal to be compatible so they fall in the rule 1. The two other rules apply only to release versions. Same with major-zero

Ah, I see. Hmn. I think, the term "release" could be mistaken by "pre-release" (as I did). I would like to avoid this. That's why I wrote in all of the documentation about "versions", not "releases".

Practically the same conditions than the major's, right?

Minor parts don't have to be higher than zero to be compatible.

I know, but for your item ""their majors are equal and higher than 0 and minor versions"" I interpret it as such. 🙂

[rafalkrupinski]

3. By release I mean not a pre-release. Pre-releases need to be equal to be compatible so they fall in the rule 1. The two other rules apply only to release versions. Same with major-zero

Ah, I see. Hmn. I think, the term "release" could be mistaken by "pre-release" (as I did). I would like to avoid this. That's why I wrote in all of the documentation about "versions", not "releases".

Perhaps you're just so immersed in this project and the lingo, that you've just read what you expected to read ;)

Releases and pre-releases have different compatibility rules, so you can't just replace 'release' with 'version', unless you write 'version other than pre-release', but to me it's just indirect way of writing 'release' 🤷🏻. Back to square one.

Practically the same conditions than the major's, right?
Minor parts don't have to be higher than zero to be compatible.
I know, but for your item ""their majors are equal and higher than 0 and minor versions"" I interpret it as such. 🙂

Missing coma perhaps? Don't let my bad grammar put you off track ;)

Hopefully this approach is clearer: #368 (comment)

[tomschr]

@rafalkrupinski @Lexicality @tlaferriere
Sorry, it took me some time to revisit this. Anything that could be considered a blocker? Otherwise I'd like to merge it to create the final semver 3 release.

Thanks for all your help!

[tomschr]

As nobody seemed to object, I'm merging it. Thank you!