Repository: python-semver/python-semver (python)

Issue:
Deprecate missing module-level functions (like semver.compare, semver.match, ...)
# Situation

We deprecated most of our module-level functions, but it seems, I forgot `semver.match`.

# Proposal

I assume, this function should also be added to the list of deprecated function. It also should be moved to the `semver.VersionInfo` class.

@python-semver/reviewers Any objections?

---

There are some other module-level functions:

* `semver.compare`
* `semver.max_ver`
* `semver.min_ver`

@gsakkis: What should we do with these? Deprecate them too and move them to the `semver.VersionInfo` class?


Pull request:
Fix #236: add missing deprecated functions
This PR fixes #236 and contains the following changes:

Deprecate:

* [x] `semver.compare` -> `semver.VersionInfo.compare`
* [x] `semver.match` -> `semver.VersionInfo.match`

Change:

* Implementation of `semver.max_ver` and `semver.min_ver`

----

~The implementation in this PR is a minimal implementation. I could have done more, but~ I would like to hear your opinion. I have two questions:

* Should we really move the functions `semver.max_ver` and `semver.min_ver` into the class? Especially if we maybe want to change them later, see #160.

* What about the "private" functions `_nat_cmp` ~and `_compare_by_keys`~? Should we move them into the class as well.

@python-semver/reviewers What do you think?


Patch:
diff --git a/semver.py b/semver.py
index de72f06..a5739b1 100644
--- a/semver.py
+++ b/semver.py
@@ -371,29 +371,80 @@ build='build.10')
         build = cls._increment_string(self._build or (token or "build") + ".0")
         return cls(self._major, self._minor, self._patch, self._prerelease, build)
 
+    def compare(self, other):
+        """
+        Compare self with other.
+
+        :param other: the second version (can be string, a dict, tuple/list, or
+             a VersionInfo instance)
+        :return: The return value is negative if ver1 < ver2,
+             zero if ver1 == ver2 and strictly positive if ver1 > ver2
+        :rtype: int
+
+        >>> semver.VersionInfo.parse("1.0.0").compare("2.0.0")
+        -1
+        >>> semver.VersionInfo.parse("2.0.0").compare("1.0.0")
+        1
+        >>> semver.VersionInfo.parse("2.0.0").compare("2.0.0")
+        0
+        >>> semver.VersionInfo.parse("2.0.0").compare(dict(major=2, minor=0, patch=0))
+        0
+        """
+        cls = type(self)
+        if isinstance(other, str):
+            other = cls.parse(other)
+        elif isinstance(other, dict):
+            other = cls(**other)
+        elif isinstance(other, (tuple, list)):
+            other = cls(*other)
+        elif not isinstance(other, cls):
+            raise TypeError(
+                "Expected str or {} instance, but got {}".format(
+                    cls.__name__, type(other)
+                )
+            )
+
+        v1 = self.to_tuple()[:3]
+        v2 = other.to_tuple()[:3]
+        x = cmp(v1, v2)
+        if x:
+            return x
+
+        rc1, rc2 = self.prerelease, other.prerelease
+        rccmp = _nat_cmp(rc1, rc2)
+
+        if not rccmp:
+            return 0
+        if not rc1:
+            return 1
+        elif not rc2:
+            return -1
+
+        return rccmp
+
     @comparator
     def __eq__(self, other):
-        return _compare_by_keys(self.to_dict(), _to_dict(other)) == 0
+        return self.compare(other) == 0
 
     @comparator
     def __ne__(self, other):
-        return _compare_by_keys(self.to_dict(), _to_dict(other)) != 0
+        return self.compare(other) != 0
 
     @comparator
     def __lt__(self, other):
-        return _compare_by_keys(self.to_dict(), _to_dict(other)) < 0
+        return self.compare(other) < 0
 
     @comparator
     def __le__(self, other):
-        return _compare_by_keys(self.to_dict(), _to_dict(other)) <= 0
+        return self.compare(other) <= 0
 
     @comparator
     def __gt__(self, other):
-        return _compare_by_keys(self.to_dict(), _to_dict(other)) > 0
+        return self.compare(other) > 0
 
     @comparator
     def __ge__(self, other):
-        return _compare_by_keys(self.to_dict(), _to_dict(other)) >= 0
+        return self.compare(other) >= 0
 
     def __repr__(self):
         s = ", ".join("%s=%r" % (key, val) for key, val in self.to_dict().items())
@@ -424,6 +475,53 @@ build='build.10')
         cls = type(self)
         return cls(self.major, self.minor, self.patch)
 
+    def match(self, match_expr):
+        """
+        Compare self to match a match expression.
+
+        :param str match_expr: operator and version; valid operators are
+              <   smaller than
+              >   greater than
+              >=  greator or equal than
+              <=  smaller or equal than
+              ==  equal
+              !=  not equal
+        :return: True if the expression matches the version, otherwise False
+        :rtype: bool
+
+        >>> semver.VersionInfo.parse("2.0.0").match(">=1.0.0")
+        True
+        >>> semver.VersionInfo.parse("1.0.0").match(">1.0.0")
+        False
+        """
+        prefix = match_expr[:2]
+        if prefix in (">=", "<=", "==", "!="):
+            match_version = match_expr[2:]
+        elif prefix and prefix[0] in (">", "<"):
+            prefix = prefix[0]
+            match_version = match_expr[1:]
+        else:
+            raise ValueError(
+                "match_expr parameter should be in format <op><ver>, "
+                "where <op> is one of "
+                "['<', '>', '==', '<=', '>=', '!=']. "
+                "You provided: %r" % match_expr
+            )
+
+        possibilities_dict = {
+            ">": (1,),
+            "<": (-1,),
+            "==": (0,),
+            "!=": (-1, 1),
+            ">=": (0, 1),
+            "<=": (-1, 0),
+        }
+
+        possibilities = possibilities_dict[prefix]
+        cmp_res = self.compare(match_version)
+
+        return cmp_res in possibilities
+
     @staticmethod
     def parse(version):
         """
@@ -495,14 +593,6 @@ prerelease='pre.2', build='build.4')
             return False
 
 
-def _to_dict(obj):
-    if isinstance(obj, VersionInfo):
-        return obj.to_dict()
-    elif isinstance(obj, tuple):
-        return VersionInfo(*obj).to_dict()
-    return obj
-
-
 @deprecated(replace="semver.VersionInfo.parse", version="2.10.0")
 def parse_version_info(version):
     """
@@ -560,25 +650,7 @@ def _nat_cmp(a, b):
         return cmp(len(a), len(b))
 
 
-def _compare_by_keys(d1, d2):
-    for key in ["major", "minor", "patch"]:
-        v = cmp(d1.get(key), d2.get(key))
-        if v:
-            return v
-
-    rc1, rc2 = d1.get("prerelease"), d2.get("prerelease")
-    rccmp = _nat_cmp(rc1, rc2)
-
-    if not rccmp:
-        return 0
-    if not rc1:
-        return 1
-    elif not rc2:
-        return -1
-
-    return rccmp
-
-
+@deprecated(version="2.10.0")
 def compare(ver1, ver2):
     """
     Compare two versions strings.
@@ -596,13 +668,11 @@ def compare(ver1, ver2):
     >>> semver.compare("2.0.0", "2.0.0")
     0
     """
-
-    v1 = VersionInfo.parse(ver1).to_dict()
-    v2 = VersionInfo.parse(ver2).to_dict()
-
-    return _compare_by_keys(v1, v2)
+    v1 = VersionInfo.parse(ver1)
+    return v1.compare(ver2)
 
 
+@deprecated(version="2.10.0")
 def match(version, match_expr):
     """
     Compare two versions strings through a comparison.
@@ -623,33 +693,8 @@ def match(version, match_expr):
     >>> semver.match("1.0.0", ">1.0.0")
     False
     """
-    prefix = match_expr[:2]
-    if prefix in (">=", "<=", "==", "!="):
-        match_version = match_expr[2:]
-    elif prefix and prefix[0] in (">", "<"):
-        prefix = prefix[0]
-        match_version = match_expr[1:]
-    else:
-        raise ValueError(
-            "match_expr parameter should be in format <op><ver>, "
-            "where <op> is one of "
-            "['<', '>', '==', '<=', '>=', '!=']. "
-            "You provided: %r" % match_expr
-        )
-
-    possibilities_dict = {
-        ">": (1,),
-        "<": (-1,),
-        "==": (0,),
-        "!=": (-1, 1),
-        ">=": (0, 1),
-        "<=": (-1, 0),
-    }
-
-    possibilities = possibilities_dict[prefix]
-    cmp_res = compare(version, match_version)
-
-    return cmp_res in possibilities
+    ver = VersionInfo.parse(version)
+    return ver.match(match_expr)
 
 
 def max_ver(ver1, ver2):
@@ -664,9 +709,10 @@ def max_ver(ver1, ver2):
     >>> semver.max_ver("1.0.0", "2.0.0")
     '2.0.0'
     """
-    cmp_res = compare(ver1, ver2)
-    if cmp_res == 0 or cmp_res == 1:
-        return ver1
+    ver1 = VersionInfo.parse(ver1)
+    cmp_res = ver1.compare(ver2)
+    if cmp_res >= 0:
+        return str(ver1)
     else:
         return ver2
 
@@ -683,9 +729,10 @@ def min_ver(ver1, ver2):
     >>> semver.min_ver("1.0.0", "2.0.0")
     '1.0.0'
     """
-    cmp_res = compare(ver1, ver2)
-    if cmp_res == 0 or cmp_res == -1:
-        return ver1
+    ver1 = VersionInfo.parse(ver1)
+    cmp_res = ver1.compare(ver2)
+    if cmp_res <= 0:
+        return str(ver1)
     else:
         return ver2
 


Test patch:
diff --git a/test_semver.py b/test_semver.py
index 4611d77..5daf3f1 100644
--- a/test_semver.py
+++ b/test_semver.py
@@ -855,8 +855,10 @@ def test_should_versioninfo_isvalid():
         (bump_minor, ("1.2.3",), {}),
         (bump_patch, ("1.2.3",), {}),
         (bump_prerelease, ("1.2.3",), {}),
+        (compare, ("1.2.1", "1.2.2"), {}),
         (format_version, (3, 4, 5), {}),
         (finalize_version, ("1.2.3-rc.5",), {}),
+        (match, ("1.0.0", ">=1.0.0"), {}),
         (parse, ("1.2.3",), {}),
         (parse_version_info, ("1.2.3",), {}),
         (replace, ("1.2.3",), dict(major=2, patch=10)),