Description
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 typeint
orstr
. 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 isNone
(allowed) or a of typeint
orstr
(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.
Thetyped_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 theVersion
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?