Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix comparison #104

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ Python SemVer library
All notable changes to this code base will be documented in this file,
in every released version.

Version 2.8.x
=============
:Released: 20yy-mm-dd
:Maintainer: Sébastien Celles <s.celles@gmail.com>

* Issue #102 (PR #...). Fix comparison between VersionInfo and tuple
* Issue #103 (PR #...). Disallow comparison between VersionInfo and string (and int)

Version 2.8.1
=============
:Released: 2018-07-09
Expand Down
29 changes: 16 additions & 13 deletions semver.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class VersionInfo(object):
"""
__slots__ = ('_major', '_minor', '_patch', '_prerelease', '_build')

def __init__(self, major, minor, patch, prerelease=None, build=None):
def __init__(self, major, minor=0, patch=0, prerelease=None, build=None):
self._major = major
self._minor = minor
self._patch = patch
Expand Down Expand Up @@ -125,33 +125,27 @@ def _asdict(self):
))

def __eq__(self, other):
if not isinstance(other, (VersionInfo, dict)):
return NotImplemented
_ensure_is_comparable(other)
return _compare_by_keys(self._asdict(), _to_dict(other)) == 0

def __ne__(self, other):
if not isinstance(other, (VersionInfo, dict)):
return NotImplemented
_ensure_is_comparable(other)
return _compare_by_keys(self._asdict(), _to_dict(other)) != 0

def __lt__(self, other):
if not isinstance(other, (VersionInfo, dict)):
return NotImplemented
_ensure_is_comparable(other)
return _compare_by_keys(self._asdict(), _to_dict(other)) < 0

def __le__(self, other):
if not isinstance(other, (VersionInfo, dict)):
return NotImplemented
_ensure_is_comparable(other)
return _compare_by_keys(self._asdict(), _to_dict(other)) <= 0

def __gt__(self, other):
if not isinstance(other, (VersionInfo, dict)):
return NotImplemented
_ensure_is_comparable(other)
return _compare_by_keys(self._asdict(), _to_dict(other)) > 0

def __ge__(self, other):
if not isinstance(other, (VersionInfo, dict)):
return NotImplemented
_ensure_is_comparable(other)
return _compare_by_keys(self._asdict(), _to_dict(other)) >= 0

def __repr__(self):
Expand Down Expand Up @@ -184,6 +178,8 @@ def parse(version):
def _to_dict(obj):
if isinstance(obj, VersionInfo):
return obj._asdict()
elif isinstance(obj, tuple):
return VersionInfo(*obj)._asdict()
return obj


Expand Down Expand Up @@ -261,6 +257,13 @@ def _compare_by_keys(d1, d2):
return rccmp


def _ensure_is_comparable(other):
comparable_types = (VersionInfo, dict, tuple)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ppkt made a good point about using interfaces in place of dict/tuple. This will allow us to compare with dict-like / tuple-like objects for free (well, not really, but nothing to care about).

The only thing is str - it's Iterable, but unlikely *str will give us what we really expected from it.

if not isinstance(other, comparable_types):
raise NotImplementedError("other type %r must be in %r"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.python.org/3/library/constants.html#NotImplemented

You need to return NotImplemented here. Python itself will raise proper TypeError if objects aren't comparable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

% (type(other), comparable_types))


def compare(ver1, ver2):
"""Compare two versions

Expand Down
24 changes: 24 additions & 0 deletions test_semver.py
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,30 @@ def test_should_compare_version_dictionaries():
assert not(v1 == v4)


def test_should_compare_version_tuples():
v1 = VersionInfo(major=3, minor=4, patch=5,
prerelease='pre.2', build='build.4')
assert v1 > (1, 0, 0)
assert v1 > (1, 0)
assert v1 > (1,)
assert v1 > (1, 0, 0, 'pre.2')
assert v1 > (1, 0, 0, 'pre.2', 'build.4')


def test_should_not_allow_to_compare_version_with_string():
v1 = VersionInfo(major=3, minor=4, patch=5,
prerelease='pre.2', build='build.4')
with pytest.raises(NotImplementedError) as e_info: # noqa
v1 > "1.0.0"


def test_should_not_allow_to_compare_version_with_int():
v1 = VersionInfo(major=3, minor=4, patch=5,
prerelease='pre.2', build='build.4')
with pytest.raises(NotImplementedError) as e_info: # noqa
v1 > 1


def test_should_compare_prerelease_with_numbers_and_letters():
v1 = VersionInfo(major=1, minor=9, patch=1, prerelease='1unms', build=None)
v2 = VersionInfo(major=1, minor=9, patch=1, prerelease=None, build='1asd')
Expand Down