Make Version properties change from read-only to read-write?

[tomschr]

Situation

After having invested a lot of time in #317 with __init__() and validating our data, I did some research and here is another of my crazy ideas. 😄 Sorry if the text is a bit longer, but I think it could be worth.

It might simplify __init__() but it would change the way our properties are handled a bit.

According to the section Accessing Parts of a Version Through Names in our documentation, the Version class contains the properties major, minor etc.

However, at the time of writing, these properties can be read only. They do not allow write access:

>>> v = Version.parse("3.4.5-pre.2+build.4")
>>> v.minor = 5
Traceback (most recent call last):
...
AttributeError: attribute 'minor' is readonly

The Idea

I'm wondering if this behavior should be changed. 🤔 Is that still contemporary? I can't exactly remember why we did that. 😉

Maybe our users have a use case which they would like to write something like this:

>>> v.major = 4
>>> v.minor = "10"

Why shouldn't they? Would that make sense?

Of course, the assignment should only be successful if we pass the right type. Invalid values are still rejected and would lead to an exception:

>>> v.major = "x"
Traceback (most recent call last):
...
ValueError: major allows only integers or strings that can be converted to integers.

The Implementation

The idea is to start with some validating functions. We distinguish basically between two types:

• the main parts consists of major, minor, and patch.
  They must be of type int or str. Furthermore, if it is an integer, the value must be greater than zero.
• the "other" parts are prerelease and build.
  Here we need to check if it is None (allowed) or a of type int or str (also allowed).

We end up with this:

def validate_mainparts(value: Union[str, int]):
    """
    Validate arguments for major, minor, or patch.

    :param value: the value.
        * Must be of type int, str.
        * If it's an int, value must be value > 0
    :return: Nothing, if everything is okay, otherwise raise exceptions.
    """
    if not isinstance(value, (int, str)):
        raise TypeError(f"The value {value} has wrong type {type(value)}. "
                        "Expected int or str.")
    value = int(value)  # could also raise an exception
    if value < 0:
        raise ValueError(f"The value {value} cannot be negative. "
                         "A version is always positive.")

def validate_otherparts(value: Optional[Union[str, int]]):
    """
    Validate arguments for prerelease and build.

    :param value: the value. Can be None, int, or str.
    :return: Nothing, if everything is okay, otherwise raise TypeError exception.
    """
    if isinstance(value, (int, str, type(None))):
        return
    raise TypeError(f"The value {value} has wrong type {type(value)}. "
                        "Expected int or str.")

Furthermore, we need a decorator/function that creates this property automatically for us. That decorator needs to be called with a name (like major), a callable to check the value (see above), and another callable to convert it to the target type. One implementation could look like:

def typed_property(name: str, validate: Callable, convert: Callable):
    """
    Create properties for version parts.

    :param name: The name of the property.
    :param validate: a function which validates the value that is going to be set.
        Returns nothing if all is good, otherwise raise ValueError or TypeError
        exceptions if the value couldn't be validated.
    :param: convert: a function which converts the input data name to the target
        type.
    :return: the property
    """
    private_name = f"_{name}"

    def fget(self):
        return getattr(self, private_name)

    def fset(self, value):
        validate(value)  # May raise exceptions here
        setattr(self, private_name, convert(value))  # all fine, so set the private name

    return property(
        fget=fget, fset=fset, doc=f"""The {name} part of a version (read and write)."""
    )

To combine it in our class, we need to add it at the class level:

class Version:
    """ ... """
    major = typed_property("major", validate_mainparts, int)
    minor = typed_property("minor", validate_mainparts, int)
    patch = typed_property("patch", validate_mainparts, int)
    prerelease = typed_property("prerelease", validate_otherparts, lambda x: x if x is None else str(x))
    build = typed_property("build", validate_otherparts, lambda x: x if x is None else str(x))

   # ... no other @property...

---

I see here some benefits:

• We don't need to repeat code. All is hidden inside typed_property and the validating functions.
• The initializer __init__ is free from validating the inputs. This is all done in the validating functions. Should simplify the initializer method a bit.

I see also some design issues:

• Having two property names.
  The typed_property decorator needs to have the property name twice, as an assignment on left side and as a string in the argument. It's probably not much, but it feels a bit un-pythonic.

• Need two callables for the typed_property decorator.
  Although this is not much either, it is also feels a bit awkward, especially with the lambda for prerelease and build.

• Validating functions outside the class
  Currently, the validating functions are outside of the Version class. I see currently no solution to integrate that into the class as that a forward reference would be needed. Theoretically, we could do that in __init__ (or even __new__) but that would complicate the matter.

Going Beyond (Descriptor Protocol)

The above issues could be solved. Python has for this kind of problems the descriptor protocol.

Properties are just an implementation of the more general descriptor protocol. That is a class which contains one of the methods __get__, __set__ or __delete__, see https://docs.python.org/3/howto/descriptor.html

Theoretically, we could add an additional class which implements the above behavior in the said methods. This would be a charming way as all the checking/validating is hidden inside the class.

However, I'm not sure to introduce another class just to implement that. It seems also a bit of an overkill to me, although it's quite pythonic. Not sure if we would introduce other problems as it raises the complexity a bit.
On the other side, it could be beneficial if we (or another user) want to derive the Version class with some other validating parts.

Related Information

• https://realpython.com/python-descriptors/
• https://www.datacamp.com/community/tutorials/python-descriptors
• https://deepsource.io/blog/demystifying-python-descriptor-protocol/

---

@python-semver/reviewers @tlaferriere Any comments? Thoughts?

[tomschr]

Regarding the descriptor protocol, I've played around a bit and tried it with a Validator class as described in the Howto link. Here is a short proof of concept:

from abc import ABC, abstractmethod

class Validator(ABC):
    def __set_name__(self, owner, name):
        self.private_name = '_' + name

    def __get__(self, obj, objtype=None):
        return getattr(obj, self.private_name)

    def __set__(self, obj, value):
        self.validate(value)
        value = self.convert(value)
        setattr(obj, self.private_name, value)

    @abstractmethod
    def validate(self, value):
        raise NotImplementedError

    @abstractmethod
    def convert(self, value):
       raise NotImplementedError


class MainPartValidator(Validator):
    def validate(self, value):
        if not isinstance(value, (int, str)):
            raise TypeError(f"The value {value} has wrong type {type(value)}. "
                        "Expected int or str.")
        value = int(value)  # could also raise an exception
        if value < 0:
            raise ValueError(f"The value {value} cannot be negative. "
                         "A version is always positive.")
    def convert(self, value):
        return int(value)

class OtherPartValidator(Validator):
    def validate(self, value):
        if value is None:
            return True
        if not isinstance(value, (int, str)):
            raise TypeError(f"The value {value} has wrong type {type(value)}. "
                        "Expected int or str.")
    def convert(self, value):
        return value if value is None else str(value)

class Version:
    major = MainPartValidator() 
    minor = MainPartValidator() 
    patch = MainPartValidator() 
    prerelease = OtherPartValidator() 
    build = OtherPartValidator()

    def __init__(self, major=0, minor=0, patch=0, prerelease=None, build=None): 
        self.major = major 
        self.minor = minor 
        self.patch = patch 
        self.prerelease = prerelease 
        self.build = build

    def to_dict(self): 
        return dict(major=self.major, minor=self.minor, patch=self.patch,
                    prerelease=self.prerelease, build=self.build)

    def __repr__(self): 
        s = ", ".join("%s=%r" % (key, val) for key, val in self.to_dict().items())
        return "%s(%s)" % (type(self).__name__, s)

This can be used like this:

>>> v1 = Version()
>>> v1
Version(major=0, minor=0, patch=0, prerelease=None, build=None)
>>> v1.major = 4
>>> v1.minor = "5"                                                                                                                                
Version(major=4, minor=5, patch=0, prerelease=None, build=None)
>>> v1.patch = 5.2
Traceback (most recent call last)
...
TypeError: The value 5.2 has wrong type <class 'float'>. Expected int or str.

However, you still have access to it like with the @property decorator:

>>> v1.major, v1.minor
(4, 5)

This didn't change and that is backward compatible.

It would be nice if we could only have one class, but due to the different nature between the main and other parts, I doubt it would be useful.