Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Add __getitem__ method to allow easy access to version parts #138

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.

Sign up for GitHub

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

Closed
wants to merge 13 commits into from
Closed

Add __getitem__ method to allow easy access to version parts #138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f1d7cde
Add __getitem__ method to allow easy access to version parts
tlaferriere Aug 16, 2019
ba5899e
Add unit tests for index access to version parts
tlaferriere Aug 19, 2019
e88e0ba
Remove whitespace on blank line
tlaferriere Aug 19, 2019
c30d38f
Merge remote-tracking branch 'kbx/master'
tlaferriere Oct 1, 2019
26a7e03
Merge upstream into fork and fix conflicts
tlaferriere Oct 8, 2019
80da20c
Fix stash conflicts and create more test vectors. Also add private ut…
tlaferriere Oct 8, 2019
a610c22
Use simpler implementation that allows negative indices and remove no…
tlaferriere Oct 8, 2019
95e79d8
Add param and raises to __getitem__ docstring
tlaferriere Oct 10, 2019
ab35e38
Merge branch 'master' of github.com:k-bx/python-semver
tlaferriere Nov 2, 2019
27632b7
Reiterate the logic for __getitem__ and update tests to follow the ac…
tlaferriere Nov 20, 2019
976a4ee
Update __getitem__ docstring
tlaferriere Nov 20, 2019
cf65b3d
Fix lint issues
tlaferriere Nov 20, 2019
dabebd9
Fix lint issues
tlaferriere Nov 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions 33 semver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,39 @@ def __iter__(self):
for v in self._astuple():
yield v

def __getitem__(self, index):
"""Implement getitem. If the part requested is undefined, or a part of the
range requested is undefined, it will throw an index error.
Negative indices are not supported

:param Union[int, slice] index: a positive integer indicating the
offset or a slice
:raises: IndexError, if index is beyond the range or a part is None
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

This docstring has an unusual indentation. According to PEP 257 indentation may look like:

def foo():
    """First line of comment

    Second line of comment. The longer description.
    """

(what I like less but seems to be favored by Guido & friends), or:

def foo():
    """
    First line of comment

    Second line of comment. The longer description.
    """

Same below, in the method following next.

Copy link
Member

Choose a reason for hiding this comment

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

Good point, but then the other docstrings needs to be adapted as well. Maybe we could do that in a combined issue later?

undefined_error = IndexError("Version part undefined")
negative_error = IndexError("Version index cannot be negative")
if isinstance(index, slice):
if (False if index.start is None else index.start < 0) or \
(False if index.stop is None else index.stop < 0):
raise negative_error
else:
slice_is_full = True
part = self._astuple()[index]
for i in part:
Copy link
Contributor

Choose a reason for hiding this comment

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

What is i here? Pylint would be complaining (if you ran it). Can you use a self-explanatory name?

Copy link
Member

Choose a reason for hiding this comment

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

One idea could be to replace i with item.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess I could use subpart instead.

Copy link
Member

Choose a reason for hiding this comment

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

See also the more recent comments below. 😉

if i is None:
slice_is_full = False
elif not slice_is_full:
raise undefined_error
part = tuple(filter(None, part))
else:
if index < 0:
raise negative_error
else:
part = self._astuple()[index]
if part is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

if not part:

... is probably more pythonic. Or is there a reason for an explicit None?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is in several places in the code, by the way, and could be made pythonic throughout the code.

raise undefined_error
return part

def bump_major(self):
"""Raise the major part of the version, return a new object
but leave self untouched
Expand Down
60 changes: 60 additions & 0 deletions 60 test_semver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,3 +698,63 @@ def test_should_return_versioninfo_with_replaced_parts(version,
def test_replace_raises_ValueError_for_non_numeric_values():
with pytest.raises(ValueError):
VersionInfo.parse("1.2.3").replace(major="x")


@pytest.mark.parametrize("version, index, expected", [
# Simple positive indices
("1.2.3-rc.0+build.0", 0, 1),
("1.2.3-rc.0+build.0", 1, 2),
("1.2.3-rc.0+build.0", 2, 3),
("1.2.3-rc.0+build.0", 3, "rc.0"),
("1.2.3-rc.0+build.0", 4, "build.0"),
("1.2.3-rc.0", 0, 1),
("1.2.3-rc.0", 1, 2),
("1.2.3-rc.0", 2, 3),
("1.2.3-rc.0", 3, "rc.0"),
("1.2.3", 0, 1),
("1.2.3", 1, 2),
("1.2.3", 2, 3),
])
def test_version_info_should_be_accessed_with_index(version, index, expected):
version_info = VersionInfo.parse(version)
assert version_info[index] == expected
tlaferriere marked this conversation as resolved.
Show resolved Hide resolved


@pytest.mark.parametrize("version, slice_object, expected", [
# Slice indices
("1.2.3-rc.0+build.0", slice(0, 5), (1, 2, 3, "rc.0", "build.0")),
("1.2.3-rc.0+build.0", slice(0, 4), (1, 2, 3, "rc.0")),
("1.2.3-rc.0+build.0", slice(0, 3), (1, 2, 3)),
("1.2.3-rc.0+build.0", slice(0, 2), (1, 2)),
("1.2.3-rc.0+build.0", slice(3, 5), ("rc.0", "build.0")),
("1.2.3-rc.0", slice(0, 4), (1, 2, 3, "rc.0")),
("1.2.3-rc.0", slice(0, 3), (1, 2, 3)),
("1.2.3-rc.0", slice(0, 2), (1, 2)),
("1.2.3", slice(0, 10), (1, 2, 3)),
("1.2.3", slice(0, 3), (1, 2, 3)),
("1.2.3", slice(0, 2), (1, 2)),
# Special cases
("1.2.3-rc.0+build.0", slice(3), (1, 2, 3)),
("1.2.3-rc.0+build.0", slice(0, 5, 2), (1, 3, "build.0")),
("1.2.3-rc.0+build.0", slice(None, 5, 2), (1, 3, "build.0")),
("1.2.3-rc.0+build.0", slice(5, 0, -2), ("build.0", 3)),
])
def test_version_info_should_be_accessed_with_slice_object(version,
slice_object,
expected):
version_info = VersionInfo.parse(version)
assert version_info[slice_object] == expected


@pytest.mark.parametrize("version, index", [
("1.2.3-rc.0+build.0", -1),
("1.2.3-rc.0", -1),
("1.2.3-rc.0", 4),
("1.2.3", -1),
("1.2.3", 3),
("1.2.3", 4),
])
def test_version_info_should_throw_index_error(version, index):
version_info = VersionInfo.parse(version)
with pytest.raises(IndexError):
version_info[index]
Morty Proxy This is a proxified and sanitized view of the page, visit original site.