Repository: python-semver/python-semver
Pull request #283 (2020-10-15 00:37:11)

Ensure equal versions have equal hashes
Per [the python datamodel](https://docs.python.org/3/reference/datamodel.html#object.__hash__), `a == b` implies `hash(a) == hash(b)`. The current implementation of `VersionInfo.__hash__` includes the build, which isn't used for equality.

tomschr: @sbrudenell Thanks for this contribution! Much appreciated! :+1: 

I like it. :+1: My only concern is that this change breaks compatibility with previous releases, right? Maybe this change should go into semver 3?

Futhermore, we should document this behavior somewhere in `docs/usage.rst`. Maybe in `Comparing Version` or in a separate section.
It doesn't need much. I would describe a general example and then give the specific one with a build part. That should include a text that the build part isn't taken into account.

Does that make sense to you?
sbrudenell: IMO it's a stretch to say this change breaks compatibility. Technically true, but any code relying on the current behavior is relying on the python data model being broken, which is pathological enough that I wouldn't bother supporting it. Up to you though.

Is there a separate branch for changes going into 3.x?

I'll update the docs.
tomschr: > IMO it's a stretch to say this change breaks compatibility. [...]

Yes, maybe my question was not entirely correct. It was more of a "thinking out loud" approach and to get your opinion. Probably I'm too concerned about that, maybe it's not a big issue. :wink: But it's something that I consider first before accepting any PR. So let me think about that.


> Is there a separate branch for changes going into 3.x?

At the moment this is not the case, unfortunately. I hope we can leave semver2 behind soon and start working on semver 3. For you, nothing changes at the moment. 

> I'll update the docs.

That would be great! :+1: 



Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 25a0d68..2fdfa8d 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -6,6 +6,68 @@ Change Log
 All notable changes to this code base will be documented in this file,
 in every released version.
 
+Version 2.12.0
+==============
+
+:Released:
+:Maintainer: Tom Schraitle
+
+Features
+--------
+
+n/a
+
+
+Bug Fixes
+---------
+
+* :gh:`291` (:pr:`292`): Disallow negative numbers of
+  major, minor, and patch for ``semver.VersionInfo``
+
+
+Additions
+---------
+
+n/a
+
+
+Deprecations
+------------
+
+n/a
+
+
+Version 2.11.0
+==============
+
+:Released: 2020-10-17
+:Maintainer: Tom Schraitle
+
+Features
+--------
+
+n/a
+
+
+Bug Fixes
+---------
+
+* :gh:`276` (:pr:`277`): VersionInfo.parse should be a class method
+   Also add authors and update changelog in :gh:`286`
+* :gh:`274` (:pr:`275`): Py2 vs. Py3 incompatibility TypeError
+
+
+Additions
+---------
+
+n/a
+
+
+Deprecations
+------------
+
+n/a
+
 
 Version 2.10.2
 ==============
@@ -22,8 +84,8 @@ Features
 Bug Fixes
 ---------
 
-:gh:`260` (:pr:`261`): Fixed ``__getitem__`` returning None on wrong parts
-:pr:`263`: Doc: Add missing "install" subcommand for openSUSE
+* :gh:`260` (:pr:`261`): Fixed ``__getitem__`` returning None on wrong parts
+* :pr:`263`: Doc: Add missing "install" subcommand for openSUSE
 
 
 Additions
@@ -32,7 +94,7 @@ Additions
 n/a
 
 Deprecations
---------
+------------
 
 * :gh:`160` (:pr:`264`):
     * :func:`semver.max_ver`
@@ -92,7 +154,7 @@ Additions
 
 
 Deprecations
---------
+------------
 * :gh:`225` (:pr:`229`): Output a DeprecationWarning for the following functions:
 
   - ``semver.parse``
diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index 0920203..a4b61fa 100644
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -27,6 +27,8 @@ Significant contributors
 * Ben Finney <ben+python@benfinney.id.au>
 * Carles Barrobés <carles@barrobes.com>
 * Craig Blaszczyk <masterjakul@gmail.com>
+* Damien Nadé <anvil@users.noreply.github.com>
+* Eli Bishop <eli-darkly@users.noreply.github.com>
 * George Sakkis <gsakkis@users.noreply.github.com>
 * Jan Pieter Waagmeester <jieter@jieter.nl>
 * Jelo Agnasin <jelo@icannhas.com>
diff --git a/docs/usage.rst b/docs/usage.rst
index 2f23e57..01c4485 100644
--- a/docs/usage.rst
+++ b/docs/usage.rst
@@ -45,10 +45,17 @@ creating a version:
 
 A :class:`semver.VersionInfo` instance can be created in different ways:
 
-* From a string::
+* From a string (a Unicode string in Python 2)::
 
     >>> semver.VersionInfo.parse("3.4.5-pre.2+build.4")
     VersionInfo(major=3, minor=4, patch=5, prerelease='pre.2', build='build.4')
+    >>> semver.VersionInfo.parse(u"5.3.1")
+    VersionInfo(major=5, minor=3, patch=1, prerelease=None, build=None)
+
+* From a byte string::
+
+    >>> semver.VersionInfo.parse(b"2.3.4")
+    VersionInfo(major=2, minor=3, patch=4, prerelease=None, build=None)
 
 * From individual parts by a dictionary::
 
@@ -56,6 +63,14 @@ A :class:`semver.VersionInfo` instance can be created in different ways:
     >>> semver.VersionInfo(**d)
     VersionInfo(major=3, minor=4, patch=5, prerelease='pre.2', build='build.4')
 
+  Keep in mind, the ``major``, ``minor``, ``patch`` parts has to
+  be positive.
+
+    >>> semver.VersionInfo(-1)
+    Traceback (most recent call last):
+    ...
+    ValueError: 'major' is negative. A version can only be positive.
+
   As a minimum requirement, your dictionary needs at least the ``major``
   key, others can be omitted. You get a ``TypeError`` if your
   dictionary contains invalid keys.
@@ -452,6 +467,32 @@ functions that leverage this capability; builtins including, but not limited to:
 (for examples, see :ref:`sec_max_min`) and :func:`sorted`.
 
 
+Determining Version Equality
+----------------------------
+
+Version equality means for semver, that major, minor, patch, and prerelease
+parts are equal in both versions you compare. The build part is ignored.
+For example::
+
+    >>> v = semver.VersionInfo.parse("1.2.3-rc4+1e4664d")
+    >>> v == "1.2.3-rc4+dedbeef"
+    True
+
+This also applies when a :class:`semver.VersionInfo` is a member of a set, or a
+dictionary key::
+
+    >>> d = {}
+    >>> v1 = semver.VersionInfo.parse("1.2.3-rc4+1e4664d")
+    >>> v2 = semver.VersionInfo.parse("1.2.3-rc4+dedbeef")
+    >>> d[v1] = 1
+    >>> d[v2]
+    1
+    >>> s = set()
+    >>> s.add(v1)
+    >>> v2 in s
+    True
+
+
 
 Comparing Versions through an Expression
 ----------------------------------------
diff --git a/semver.py b/semver.py
index 85811fe..7234714 100644
--- a/semver.py
+++ b/semver.py
@@ -10,7 +10,11 @@ import sys
 import warnings
 
 
-__version__ = "2.10.2"
+PY2 = sys.version_info[0] == 2
+PY3 = sys.version_info[0] == 3
+
+
+__version__ = "2.12.0"
 __author__ = "Kostiantyn Rybnikov"
 __author_email__ = "k-bx@k-bx.com"
 __maintainer__ = ["Sebastien Celles", "Tom Schraitle"]
@@ -60,6 +64,53 @@ if not hasattr(__builtins__, "cmp"):
         return (a > b) - (a < b)
 
 
+if PY3:  # pragma: no cover
+    string_types = str, bytes
+    text_type = str
+    binary_type = bytes
+
+    def b(s):
+        return s.encode("latin-1")
+
+    def u(s):
+        return s
+
+
+else:  # pragma: no cover
+    string_types = unicode, str
+    text_type = unicode
+    binary_type = str
+
+    def b(s):
+        return s
+
+    # Workaround for standalone backslash
+    def u(s):
+        return unicode(s.replace(r"\\", r"\\\\"), "unicode_escape")
+
+
+def ensure_str(s, encoding="utf-8", errors="strict"):
+    # Taken from six project
+    """
+    Coerce *s* to `str`.
+
+    For Python 2:
+      - `unicode` -> encoded to `str`
+      - `str` -> `str`
+
+    For Python 3:
+      - `str` -> `str`
+      - `bytes` -> decoded to `str`
+    """
+    if not isinstance(s, (text_type, binary_type)):
+        raise TypeError("not expecting type '%s'" % type(s))
+    if PY2 and isinstance(s, text_type):
+        s = s.encode(encoding, errors)
+    elif PY3 and isinstance(s, binary_type):
+        s = s.decode(encoding, errors)
+    return s
+
+
 def deprecated(func=None, replace=None, version=None, category=DeprecationWarning):
     """
     Decorates a function to output a deprecation warning.
@@ -144,7 +195,7 @@ def comparator(operator):
 
     @wraps(operator)
     def wrapper(self, other):
-        comparable_types = (VersionInfo, dict, tuple, list, str)
+        comparable_types = (VersionInfo, dict, tuple, list, text_type, binary_type)
         if not isinstance(other, comparable_types):
             raise TypeError(
                 "other type %r must be in %r" % (type(other), comparable_types)
@@ -192,9 +243,24 @@ class VersionInfo(object):
     )
 
     def __init__(self, major, minor=0, patch=0, prerelease=None, build=None):
-        self._major = int(major)
-        self._minor = int(minor)
-        self._patch = int(patch)
+        # Build a dictionary of the arguments except prerelease and build
+        version_parts = {
+            "major": major,
+            "minor": minor,
+            "patch": patch,
+        }
+
+        for name, value in version_parts.items():
+            value = int(value)
+            version_parts[name] = value
+            if value < 0:
+                raise ValueError(
+                    "{!r} is negative. A version can only be positive.".format(name)
+                )
+
+        self._major = version_parts["major"]
+        self._minor = version_parts["minor"]
+        self._patch = version_parts["patch"]
         self._prerelease = None if prerelease is None else str(prerelease)
         self._build = None if build is None else str(build)
 
@@ -423,7 +489,7 @@ build='build.10')
         0
         """
         cls = type(self)
-        if isinstance(other, str):
+        if isinstance(other, string_types):
             other = cls.parse(other)
         elif isinstance(other, dict):
             other = cls(**other)
@@ -575,7 +641,7 @@ build='build.10')
         return version
 
     def __hash__(self):
-        return hash(self.to_tuple())
+        return hash(self.to_tuple()[:4])
 
     def finalize_version(self):
         """
@@ -637,8 +703,8 @@ build='build.10')
 
         return cmp_res in possibilities
 
-    @staticmethod
-    def parse(version):
+    @classmethod
+    def parse(cls, version):
         """
         Parse version string to a VersionInfo instance.
 
@@ -647,11 +713,15 @@ build='build.10')
         :raises: :class:`ValueError`
         :rtype: :class:`VersionInfo`
 
+        .. versionchanged:: 2.11.0
+           Changed method from static to classmethod to
+           allow subclasses.
+
         >>> semver.VersionInfo.parse('3.4.5-pre.2+build.4')
         VersionInfo(major=3, minor=4, patch=5, \
 prerelease='pre.2', build='build.4')
         """
-        match = VersionInfo._REGEX.match(version)
+        match = cls._REGEX.match(ensure_str(version))
         if match is None:
             raise ValueError("%s is not valid SemVer string" % version)
 
@@ -661,7 +731,7 @@ prerelease='pre.2', build='build.4')
         version_parts["minor"] = int(version_parts["minor"])
         version_parts["patch"] = int(version_parts["patch"])
 
-        return VersionInfo(**version_parts)
+        return cls(**version_parts)
 
     def replace(self, **parts):
         """
@@ -825,7 +895,7 @@ def max_ver(ver1, ver2):
     >>> semver.max_ver("1.0.0", "2.0.0")
     '2.0.0'
     """
-    if isinstance(ver1, str):
+    if isinstance(ver1, string_types):
         ver1 = VersionInfo.parse(ver1)
     elif not isinstance(ver1, VersionInfo):
         raise TypeError()
diff --git a/setup.cfg b/setup.cfg
index 1cefc4b..a22f03a 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -13,6 +13,7 @@ addopts =
 
 [flake8]
 max-line-length = 88
+ignore = F821,W503
 exclude =
     .env,
     .eggs,


Test patch:
diff --git a/test_semver.py b/test_semver.py
index b23576d..44b7572 100644
--- a/test_semver.py
+++ b/test_semver.py
@@ -73,6 +73,20 @@ def test_fordocstrings(func):
     assert func.__doc__, "Need a docstring for function %r" % func.__name
 
 
+@pytest.mark.parametrize(
+    "ver",
+    [
+        {"major": -1},
+        {"major": 1, "minor": -2},
+        {"major": 1, "minor": 2, "patch": -3},
+        {"major": 1, "minor": -2, "patch": 3},
+    ],
+)
+def test_should_not_allow_negative_numbers(ver):
+    with pytest.raises(ValueError, match=".* is negative. .*"):
+        VersionInfo(**ver)
+
+
 @pytest.mark.parametrize(
     "version,expected",
     [
@@ -679,6 +693,20 @@ def test_parse_version_info_str_hash():
     d[v] = ""  # to ensure that VersionInfo are hashable
 
 
+def test_equal_versions_have_equal_hashes():
+    v1 = parse_version_info("1.2.3-alpha.1.2+build.11.e0f985a")
+    v2 = parse_version_info("1.2.3-alpha.1.2+build.22.a589f0e")
+    assert v1 == v2
+    assert hash(v1) == hash(v2)
+    d = {}
+    d[v1] = 1
+    d[v2] = 2
+    assert d[v1] == 2
+    s = set()
+    s.add(v1)
+    assert v2 in s
+
+
 def test_parse_method_for_version_info():
     s_version = "1.2.3-alpha.1.2+build.11.e0f985a"
     v = VersionInfo.parse(s_version)
@@ -1128,3 +1156,21 @@ def test_next_version_with_versioninfo(version, part, expected):
 )
 def test_repr(version, expected):
     assert repr(version) == expected
+
+
+def test_subclass_from_versioninfo():
+    class SemVerWithVPrefix(VersionInfo):
+        @classmethod
+        def parse(cls, version):
+            if not version.startswith("v"):
+                raise ValueError(
+                    "{v}: not a valid semantic version tag".format(v=version)
+                )
+            return super(SemVerWithVPrefix, cls).parse(version[1:])
+
+        def __str__(self):
+            # Reconstruct the tag.
+            return "v" + super(SemVerWithVPrefix, self).__str__()
+
+    v = SemVerWithVPrefix.parse("v1.2.3")
+    assert str(v) == "v1.2.3"
diff --git a/test_typeerror-274.py b/test_typeerror-274.py
new file mode 100644
index 0000000..2ed03d6
--- /dev/null
+++ b/test_typeerror-274.py
@@ -0,0 +1,102 @@
+import pytest
+import sys
+
+import semver
+
+
+PY2 = sys.version_info[0] == 2
+PY3 = sys.version_info[0] == 3
+
+
+def ensure_binary(s, encoding="utf-8", errors="strict"):
+    """Coerce **s** to six.binary_type.
+
+    For Python 2:
+      - `unicode` -> encoded to `str`
+      - `str` -> `str`
+
+    For Python 3:
+      - `str` -> encoded to `bytes`
+      - `bytes` -> `bytes`
+    """
+    if isinstance(s, semver.text_type):
+        return s.encode(encoding, errors)
+    elif isinstance(s, semver.binary_type):
+        return s
+    else:
+        raise TypeError("not expecting type '%s'" % type(s))
+
+
+def test_should_work_with_string_and_unicode():
+    result = semver.compare(semver.u("1.1.0"), semver.b("1.2.2"))
+    assert result == -1
+    result = semver.compare(semver.b("1.1.0"), semver.u("1.2.2"))
+    assert result == -1
+
+
+class TestEnsure:
+    # From six project
+    # grinning face emoji
+    UNICODE_EMOJI = semver.u("\U0001F600")
+    BINARY_EMOJI = b"\xf0\x9f\x98\x80"
+
+    def test_ensure_binary_raise_type_error(self):
+        with pytest.raises(TypeError):
+            semver.ensure_str(8)
+
+    def test_errors_and_encoding(self):
+        ensure_binary(self.UNICODE_EMOJI, encoding="latin-1", errors="ignore")
+        with pytest.raises(UnicodeEncodeError):
+            ensure_binary(self.UNICODE_EMOJI, encoding="latin-1", errors="strict")
+
+    def test_ensure_binary_raise(self):
+        converted_unicode = ensure_binary(
+            self.UNICODE_EMOJI, encoding="utf-8", errors="strict"
+        )
+        converted_binary = ensure_binary(
+            self.BINARY_EMOJI, encoding="utf-8", errors="strict"
+        )
+        if semver.PY2:
+            # PY2: unicode -> str
+            assert converted_unicode == self.BINARY_EMOJI and isinstance(
+                converted_unicode, str
+            )
+            # PY2: str -> str
+            assert converted_binary == self.BINARY_EMOJI and isinstance(
+                converted_binary, str
+            )
+        else:
+            # PY3: str -> bytes
+            assert converted_unicode == self.BINARY_EMOJI and isinstance(
+                converted_unicode, bytes
+            )
+            # PY3: bytes -> bytes
+            assert converted_binary == self.BINARY_EMOJI and isinstance(
+                converted_binary, bytes
+            )
+
+    def test_ensure_str(self):
+        converted_unicode = semver.ensure_str(
+            self.UNICODE_EMOJI, encoding="utf-8", errors="strict"
+        )
+        converted_binary = semver.ensure_str(
+            self.BINARY_EMOJI, encoding="utf-8", errors="strict"
+        )
+        if PY2:
+            # PY2: unicode -> str
+            assert converted_unicode == self.BINARY_EMOJI and isinstance(
+                converted_unicode, str
+            )
+            # PY2: str -> str
+            assert converted_binary == self.BINARY_EMOJI and isinstance(
+                converted_binary, str
+            )
+        else:
+            # PY3: str -> str
+            assert converted_unicode == self.UNICODE_EMOJI and isinstance(
+                converted_unicode, str
+            )
+            # PY3: bytes -> str
+            assert converted_binary == self.UNICODE_EMOJI and isinstance(
+                converted_unicode, str
+            )