Skip to content

VersionInfo.next_version() returns Union[VersionInfo, str] #251

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

Closed
tlaferriere opened this issue May 13, 2020 · 3 comments · Fixed by #254 or #257
Closed

VersionInfo.next_version() returns Union[VersionInfo, str] #251

tlaferriere opened this issue May 13, 2020 · 3 comments · Fixed by #254 or #257
Labels
Bug Error, flaw or fault to produce incorrect or unexpected results Doc Documentation related issue

Comments

@tlaferriere
Copy link
Contributor

VersionInfo.next_version() return type is uncertain because at line 463 of semver.py return version.replace(prerelease=None, build=None) returns a VersionInfo object, whereas in line 466 it is explicitly converted to str and at line 470, version.bump_prerelease() is documented with rtype: str.
This is problematic because if you call this on a version that has a prerelease or build number, you get a VersionInfo object, and otherwise you get a str, and these two types must be handled quite differently.

Personally, I would expect VersionInfo.next_version() to return a VersionInfo object in all cases, but I think the main problem here is the ambiguity of the return type.

@tomschr
Copy link
Member

tomschr commented May 13, 2020

Thanks @tlaferriere for your report! I think this is an oversight. From my understanding, the method should always return a VersionInfo object.

We should ask @gsakkis, I guess you've intended to return a VersionInfo object all the time, am I right with this assumption?

@tomschr tomschr added the Bug Error, flaw or fault to produce incorrect or unexpected results label May 13, 2020
tomschr added a commit to tomschr/python-semver that referenced this issue May 13, 2020
* Return VersionInfo instance in VersionInfo.next_version and
  not str when part is major, minor, or patch.
* Change test_next_version_with_versioninfo
  Test for correct return type (should be VersionInfo)

Co-authored-by: Thomas Laferriere <tlaferriere@users.noreply.github.com>
@tomschr tomschr added the Doc Documentation related issue label May 13, 2020
@gsakkis
Copy link
Contributor

gsakkis commented May 13, 2020

@tomschr indeed, this must have been a leftover from the very first version that returned always a string.

@tomschr tomschr linked a pull request May 13, 2020 that will close this issue
@tomschr
Copy link
Member

tomschr commented May 13, 2020

Thanks George for conforming! 👍

I will create a 2.10.1 release soon.

@tomschr tomschr mentioned this issue May 13, 2020
tomschr added a commit to tomschr/python-semver that referenced this issue May 13, 2020
* Update CHANGELOG
* Improve release-procedure.md
* python-semver#249: Add release policy and version restriction in documentation to
  help our users which would like to stay on the major 2 release.
* python-semver#250: Simplify installation semver on openSUSE with ``obs://``.
* python-semver#251: Return type of ``semver.VersionInfo.next_version``
  to always return a ``VersionInfo`` instance.
* python-semver#256: Made docstrings consistent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Error, flaw or fault to produce incorrect or unexpected results Doc Documentation related issue
Projects
None yet
3 participants