René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Make Version properties change from read-only to read-write?","articleBody":"# Situation\r\n\r\nAfter 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. :smile:  Sorry if the text is a bit longer, but I think it could be worth.\r\n\r\nIt might simplify `__init__()` but it would change the way our properties are handled a bit.\r\n\r\nAccording to the section [Accessing Parts of a Version Through Names](https://python-semver.readthedocs.io/en/latest/usage.html#accessing-parts-of-a-version-through-names) in our documentation, the `Version` class contains the properties `major`, `minor` etc.\r\n\r\nHowever, at the time of writing, these properties can be read only. They do not allow write access:\r\n\r\n```python\r\n\u003e\u003e\u003e v = Version.parse(\"3.4.5-pre.2+build.4\")\r\n\u003e\u003e\u003e v.minor = 5\r\nTraceback (most recent call last):\r\n...\r\nAttributeError: attribute 'minor' is readonly\r\n```\r\n\r\n# The Idea\r\n\r\nI'm wondering if this behavior should be changed. :thinking:  Is that still contemporary? I can't exactly remember why we did that. :wink: \r\n\r\nMaybe our users have a use case which they would like to write something like this:\r\n\r\n```python\r\n\u003e\u003e\u003e v.major = 4\r\n\u003e\u003e\u003e v.minor = \"10\"\r\n```\r\n\r\nWhy shouldn't they? Would that make sense?\r\n\r\nOf course, the assignment should only be successful if we pass the right type. Invalid values are still rejected and would lead to an exception:\r\n\r\n```python\r\n\u003e\u003e\u003e v.major = \"x\"\r\nTraceback (most recent call last):\r\n...\r\nValueError: major allows only integers or strings that can be converted to integers.\r\n```\r\n\r\n\r\n# The Implementation\r\n\r\nThe idea is to start with some validating functions. We distinguish basically between two types:\r\n\r\n* the **main parts** consists of major, minor, and patch. \r\n  They must be of type `int` or `str`. Furthermore, if it is an integer, the value must be greater than zero.\r\n* the **\"other\" parts** are prerelease and build.\r\n  Here we need to check if it is `None` (allowed) or a of type `int` or `str` (also allowed).\r\n\r\nWe end up with this:\r\n\r\n```python\r\ndef validate_mainparts(value: Union[str, int]):\r\n    \"\"\"\r\n    Validate arguments for major, minor, or patch.\r\n\r\n    :param value: the value.\r\n        * Must be of type int, str.\r\n        * If it's an int, value must be value \u003e 0\r\n    :return: Nothing, if everything is okay, otherwise raise exceptions.\r\n    \"\"\"\r\n    if not isinstance(value, (int, str)):\r\n        raise TypeError(f\"The value {value} has wrong type {type(value)}. \"\r\n                        \"Expected int or str.\")\r\n    value = int(value)  # could also raise an exception\r\n    if value \u003c 0:\r\n        raise ValueError(f\"The value {value} cannot be negative. \"\r\n                         \"A version is always positive.\")\r\n\r\ndef validate_otherparts(value: Optional[Union[str, int]]):\r\n    \"\"\"\r\n    Validate arguments for prerelease and build.\r\n\r\n    :param value: the value. Can be None, int, or str.\r\n    :return: Nothing, if everything is okay, otherwise raise TypeError exception.\r\n    \"\"\"\r\n    if isinstance(value, (int, str, type(None))):\r\n        return\r\n    raise TypeError(f\"The value {value} has wrong type {type(value)}. \"\r\n                        \"Expected int or str.\")\r\n```\r\n\r\nFurthermore, 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:\r\n\r\n```python\r\ndef typed_property(name: str, validate: Callable, convert: Callable):\r\n    \"\"\"\r\n    Create properties for version parts.\r\n\r\n    :param name: The name of the property.\r\n    :param validate: a function which validates the value that is going to be set.\r\n        Returns nothing if all is good, otherwise raise ValueError or TypeError\r\n        exceptions if the value couldn't be validated.\r\n    :param: convert: a function which converts the input data name to the target\r\n        type.\r\n    :return: the property\r\n    \"\"\"\r\n    private_name = f\"_{name}\"\r\n\r\n    def fget(self):\r\n        return getattr(self, private_name)\r\n\r\n    def fset(self, value):\r\n        validate(value)  # May raise exceptions here\r\n        setattr(self, private_name, convert(value))  # all fine, so set the private name\r\n\r\n    return property(\r\n        fget=fget, fset=fset, doc=f\"\"\"The {name} part of a version (read and write).\"\"\"\r\n    )\r\n```\r\n\r\nTo combine it in our class, we need to add it at the class level:\r\n\r\n```python\r\nclass Version:\r\n    \"\"\" ... \"\"\"\r\n    major = typed_property(\"major\", validate_mainparts, int)\r\n    minor = typed_property(\"minor\", validate_mainparts, int)\r\n    patch = typed_property(\"patch\", validate_mainparts, int)\r\n    prerelease = typed_property(\"prerelease\", validate_otherparts, lambda x: x if x is None else str(x))\r\n    build = typed_property(\"build\", validate_otherparts, lambda x: x if x is None else str(x))\r\n\r\n   # ... no other @property...\r\n```\r\n\r\n----\r\n\r\nI see here some benefits:\r\n\r\n* We don't need to repeat code. All is hidden inside `typed_property` and the validating functions.\r\n* The initializer `__init__` is free from validating the inputs. This is all done in the validating functions. Should simplify the initializer method a bit.\r\n\r\nI see also some design issues:\r\n\r\n* Having two property names.\r\n  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.\r\n\r\n* Need two callables for the `typed_property` decorator.\r\n  Although this is not much either, it is also feels a bit awkward, especially with the lambda for prerelease and build.\r\n\r\n* Validating functions outside the class\r\n   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.\r\n\r\n# Going Beyond (Descriptor Protocol)\r\nThe above issues could be solved. Python has for this kind of problems the descriptor protocol.\r\n\r\nProperties 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\r\n\r\nTheoretically, 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.\r\n\r\nHowever, 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.\r\nOn the other side, it could be beneficial if we (or another user) want to derive the `Version` class with some other validating parts.\r\n\r\n# Related Information\r\n\r\n* https://realpython.com/python-descriptors/\r\n* https://www.datacamp.com/community/tutorials/python-descriptors\r\n* https://deepsource.io/blog/demystifying-python-descriptor-protocol/\r\n\r\n----\r\n\r\n@python-semver/reviewers @tlaferriere  Any comments? Thoughts?\r\n\r\n","author":{"url":"https://github.com/tomschr","@type":"Person","name":"tomschr"},"datePublished":"2020-11-22T11:59:16.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":1},"url":"https://github.com/327/python-semver/issues/327"}