Rethinking __init__ of VersionInfo class?

@tomschr

Situation

Originally posted by @tomschr in #258 (comment)

From the above discussion, I thought it would be worth to decouple the compare discussion from the initializer discussion. Both are somewhat related, but can live independently.

With a more "advanced" initializer/constructor we get the following benefits:

more "pythonic": it's one, obvious way to get an instance.
avoids the longer function call Version.parse(...).
more readable

With such an (overloaded?) initializer, we could cover the following use cases:

>>> from semver import Version
>>> Version(1)
Version(major=1, minor=0, patch=0, prerelease=None, build=None)
>>> Version(1, "4", b"5")
Version(major=1, minor=4, patch=5, prerelease=None, build=None)
>>> Version(1, patch=2)
Version(major=1, minor=0, patch=2, prerelease=None, build=None)
>>> Version("1.2.3")
Version(major=1, minor=2, patch=3, prerelease=None, build=None)
>>> Version(b"1.2.3")
Version(major=1, minor=2, patch=3, prerelease=None, build=None)
>>> v = Version(b"2.3.4")
>>> Version(v)
Version(major=2, minor=3, patch=4, prerelease=None, build=None)
>>> t = (1, 2, 3)
>>> Version(*t)                                             
Version(major=1, minor=2, patch=3, prerelease=None, build=None)
>>> d = {'major': 3, 'minor': 4, 'patch': 5,  'prerelease': 'pre.2', 'build': 'build.4'}
>>> Version(**d)
Version(major=3, minor=4, patch=5, prerelease='pre.2', build='build.4')

Discussions and Possible Solutions

To implement a somewhat more advanced constructor/initializer, we have these options:

Program it manually with isinstance and if...else constructs
Use the typing.overload function (suggested by @tlaferriere)
Use functools.singledispatch

However, all three comes at a cost or an issue:

Maybe not impossible, but the code would look probably ugly.
"The @overload decorator is purely for type hints, you can only specify one function body and it has to distinguish the different types using isinstance.` (Originally posted by @tlaferriere in Consider keeping compare module level function #258 (comment))
Is not possible with an __init__ method. The singledispatch works only for functions(!), not methods. For methods we would need functools.singledispatchmethod which is only available from Python >= 3.8.

Another idea goes into a completely different direction. Maybe we shouldn't change the Version class much, but offer a much shorter variant: semver.v.

from functools import singledispatch

# ...
@singledispatch
def ver(major, minor=0, patch=0, prerelease=None, build=None) -> "Version":
    return Version(major, minor, patch, prerelease, build)
    
@ver.register(bytes)
@ver.register(str)
def _(ver: str) -> "Version":
    if isinstance(ver, bytes):
       ver = str(ver, "utf-8")
    if "." in ver:
        return Version.parse(ver)
    return Version(int(ver))

@ver.register(Version)
def _(ver: "Version") -> "Version":
    return ver

Which means, we could just use semver.v as a much shorter variant:

>>> import semver
>>> semver.v("1.2.3")
Version(major=1, minor=2, patch=3, prerelease=None, build=None)

One drawback could be that v is quite short. Maybe too short? Especially if you import it with from semver import v it could be easily overwritten by other, local variables.
That could be a bit avoided to use capital V or ver. Or we use the much longer name semver.version.

Thoughts? Any other ideas? Would that be worth the effort?

@tlaferriere @gsakkis @ppkt

I found another solution which doesn't need overloading. The previous signature of __init__ was this:

    def __init__(
        major: SupportsInt,
        minor: SupportsInt = 0,
        patch: SupportsInt = 0,
        prerelease: Union[String, int] = None,
        build: Union[String, int] = None,
        )

This changed now to this:

    def __init__(self, *args: Tuple, **kwargs: Dict):
        # print(f">> args={args} {len(args)}, kwargs={kwargs} {len(kwargs)}")
        verlist = [None, None, None, None, None]

        if not args and not kwargs:
            raise ValueError("No argument given.")

        if len(args) > 5:
            raise ValueError("You cannot pass more than 5 arguments to Version")

        if args and "." in str(args[0]):
            # we have a version string as first argument
            cls = self.__class__
            v = cls._parse(args[0])
            self._major = int(v["major"])
            self._minor = int(v["minor"])
            self._patch = int(v["patch"])
            self._prerelease = v["prerelease"]
            self._build = v["build"]
            return

        for index, item in enumerate(args):
            verlist[index] = args[index]

        # Build a dictionary of the arguments except prerelease and build
        try:
            version_parts = {
                # Prefer kwargs over args
                "major": int(kwargs.pop("major", 0) or verlist[0] or 0),
                "minor": int(kwargs.pop("minor", 0) or verlist[1] or 0),
                "patch": int(kwargs.pop("patch", 0) or verlist[2] or 0)
            }
        except ValueError:
            raise ValueError("Expected integer or integer string for major, "
                             "minor, or patch")

        prerelease = kwargs.pop("prerelease", None)
        build = kwargs.pop("build", None)
        # Check, if kwargs contains other unknown keys:
        if kwargs:
            raise ValueError("Unknown key passed in {n}: {k}".format(
                n=self.__class__.__name__,
                k=", ".join([f"{k}={v}"
                             for k, v in kwargs.items()])))

        for name, value in version_parts.items():
            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)

This allows some interesting variations:

>>> import semver
>>> semver.Version(1, 2, 3)
Version(major=1, minor=2, patch=3, prerelease=None, build=None)
>>> semver.Version("1.2.3")
Version(major=1, minor=2, patch=3, prerelease=None, build=None)
>>> semver.Version(1, minor="2", patch=3)
Version(major=1, minor=2, patch=3, prerelease=None, build=None)

You can also use positional arguments and keyword arguments to "overwrite" one or the other:

>>> semver.Version(1, major="2", patch=3)
Version(major=2, minor=0, patch=3, prerelease=None, build=None)

Probably the code can be improved, but it's just a proof-of-concept to show this idea. Additionally, it avoids the complicated overload code. What do you think?

Fix python-semver#303: Fix Version.__init__ method

If the above PR got accepted, the Version.parse would become obsolete. In that case, I would suggest to deprecate it.

Fix python-semver#303: Fix Version.__init__ method

Navigation Menu

Repository navigation

Rethinking init of VersionInfo class? #303

Situation

Discussions and Possible Solutions

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Rethinking __init__ of VersionInfo class? #303

Description

Situation

Discussions and Possible Solutions

Activity

tomschr commented on Nov 5, 2020

tomschr commented on Nov 6, 2020

Metadata

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Issue actions

Rethinking init of VersionInfo class? #303