Better version comparison

[romanroibu]

This PR ensures all comparisons of version numbers are done with parsed version strings, and using the best practices.

[romanroibu]

@papr @pfaion There is an open question: version_utils is using distutils.version for parsing version strings, but pupil_recording is using packaging.version internally to parse and compare version strings. Which one of these do we want to choose and use everywhere?

[papr]

Please use packaging.version.parse

[pfaion]

@papr why do we use packaging.version.parse instead of packaging.version.Version?
It feels like the special case with LegacyVersions being sorted strictly below Version could potentially cause headaches. Are there any versions in our ecosystem that are not PEP440 compatible?

[pfaion]

@romanroibu I'm honestly not sure, if SemanticVersion is a better name here.
This strongly suggests versioning according to the Semantic Versioning scheme, but this is specifically not the case here as packaging.version.parse() will strictly order non-PEP440-compliant < PEP440-compliant, which is not the case for "semantic versions".

[pfaion]

Specifically, Semantic Versioning is not fully compatible with PEP440: https://www.python.org/dev/peps/pep-0440/#semantic-versioning

[romanroibu]

I also considered PupilVersion, but that didn't seem better since it's confusing without an explanation; it's similar reasoning to why I think Version is a bad name. I agree that Semantic Version as a name means a specific format, but it seemed the closest to what this is - which is a formatted version that caries specific meaning and can be compared. If you can think of better names, please let me know.

[romanroibu]

I'd be open to calling it PEP440Version, but as @papr mentioned, not all of the versions we're using are PEP440-compatible.

[pfaion]

I agree nothing is optimal here. I'm still angry at the Python folks for making PEP440 essentially a subset of SemVer, thereby creating so many problems. At least we are not creating our own versioning scheme again but rely on the result of packaging's parse(), which is at least well defined.

For this reason, I also don't like PupilVersion, because it exactly sounds like we built our own versioning scheme.

For simplicity, I'd probably favor Version and put a comment explaining or referencing the packaging parse() format nearby. So if someone is confused (which probably cannot be avoided anyways when talking about versioning schemes in Python), they can at least find information when looking up the definition.

[pfaion]

It's sad that parse() outputs such a weird scheme again, as you would have to come up with something like PEP440VersionWithLegacyFallback to describe it appropriately. But if we really only use this one version type inside of Pupil, I think just calling it Version is fine, as we don't need to differentiate between different version schemes (hopefully).

[papr]

How about ParsedVersion? This would make it easy to find, differentiates from packaging.version.Version, indicates that it is the output of parse(), and does not make any assumptions about the actual type of version it contains.

[pfaion]

I'd be fine with ParsedVersion, although I'd still prefer simply Version as long as we're not working with multiple different versioning formats in parallel.

I mean either you don't think about it at all if you come across it and assume PEP440 (which will just work fine), or you are aware of the hassle in Python and look at where Version is imported from and you will immediately find the necessary information. Calling it ParsedVersion might make you look up what this is about either way, which might or might not be what we want 😄

[pfaion]

This PR requires having the packaging library at version >= 20.0, otherwise, you get the following error on Windows and macOS:

_pickle.PicklingError: Can't pickle <class 'packaging._structures.Infinity'>: it's not the same object as packaging._structures.Infinity

If you got this error, please update packaging with:

pip install -U packaging