mxcube / mxcubecore

Backend used by MXCuBE
http://mxcube.github.io/mxcube/
GNU Lesser General Public License v3.0
11 stars 52 forks source link

Type annotations and Python version #775

Closed rhfogh closed 2 months ago

rhfogh commented 1 year ago

We ought to start using type annotations (well, I ought to). Looking at the Python documentation, the syntax for tyupe annotations seems to change from Python 3.9.

Could we move to 3.9 as the minimum version,? What might the problems be?

oldfielj-ansto commented 1 year ago

I think the bulk of the typing improvements did come in with Python3.9, but most of these are available in the backport library typing-extensions.

The only real difference typehinting in Python3.9 rather than Python3.8 is the introduction of Type Hinting Generics in Standard Collections.

What that means in practice is that in Python3.8 and below your code might look like this:

from typing import List, Tuple

def pointless_task(value: List[str]) -> Tuple[str, ...]:
    return tuple(value)

And in Python3.9 your code would instead look like this:

def pointless_task(value: list[str]) -> tuple[str, ...]:
    return tuple(value)

The only other feature that isn't supported by previous versions of Python using the backport module is the Type Union Operator introduced in Python3.10.

Where typehints prior to Python3.10 might have been written like this:

import random
import secrets
from typing import Union, Literal

def unreliable_task() -> Union[str, None]:
    _flip: Literal[0, 1] = random.randint(0, 1)
    if _flip:
        return secrets.token_hex(15)
    return None

Code from Python3.10+ would look like this:

import random
import secrets
from typing import Union, Literal

def unreliable_task() -> str | None:
    _flip: Literal[0, 1] = random.randint(0, 1)
    if _flip:
        return secrets.token_hex(15)
    return None

Newer features such as the Self type, introduce in Python3.11, can be used by way of the typing-extensions library I mentioned above.

So if I wanted to use the Self type to typehint code written in Python3.10 and below, I would just import it from the backport library:

from typing import Any, Dict
from typing_extensions import Self

class User:
    def __init__(self, name: str) -> None:
        self.name = name

    @classmethod
    def fromdict(cls, value: Dict[str, Any]) -> Self:
        if value.get("name") is None:
            raise ValueError("Name required in value!")
        return cls(name=value["name"])

As for any problems making Python3.9 the minimum supported version, I don't see any beyond possibly being annoying to deploy on older systems that may not have Python3.9 installed.

marcus-oscarsson commented 1 year ago

@oldfielj-ansto thanks for the nice summary. @rhfogh Its true that I have not thought about these differences my self.

The type annotation syntax came with Python 3.5 (I think). At the moment we are mostly using the typehints for documentation and to some extent for introspection but we are so far not using them for runtime checks. I think we are assuming Python 3.9 with our current usage of typehints but we have not explicitly said so.

rhfogh commented 1 year ago

@oldfielj-ansto Thanks - that gave me a much needed update. I keep telling myself I ought to start using type hints properly, but since we are likely to upgrade to a higher minimum Python version at some point soon, I would rather wait till that was done so we do not have to rewrite from the old syntax later. So I'd suggest putting 'Minimunm Python version' on the agenda of the next developers' meeting so we can agree on versions and times.

beteva commented 1 year ago

@rhfogh , the problem with a minimum/maximum python version could come outside MXCuBE. E.g. at ESRF for time being we cannot go for a higher than 3.9 version due to some bliss (our control system) dependencies. So I guess on the agenda should be not only minimum, but also maximum version.

oldfielj-ansto commented 1 year ago

@beteva Maximum versions aren't really a concern, we run tests for each supported Python version. For the most part, any code that runs on Python3.6, should run just as well on all versions up to and including Python3.11. The main exception here being with different packages dropping support for out of support versions of Python or developing bugs with newer versions.

My position would be that we should stick to developing MXCuBE for the oldest version of Python still in active support, right now that's Python3.8, where practical we should also attempt to ensure compatibility with the latest version of Python.

rhfogh commented 1 year ago

@oldfielj-ansto I guess you win that argument. I must admit that I do rather crave the nice syntax of Python 9 and 10 (additional types - not so much), so I shall feel rather less urgency in adding type annotations while you still need to do 'import List' and 'Union[str, None]'. But maybe we'll get to it one day.

If there is no more activity on this issue over the next couple of days I shall close it.

marcus-oscarsson commented 1 year ago

Sounds good, I also agree with what @oldfielj-ansto said above. If its what we agree we should perhaps just make sure that we are using typing-extensions, right ?

oldfielj-ansto commented 1 year ago

@marcus-oscarsson As a general rule of thumb, I'd say default to importing the type you need from typing-extensions, unless you're sure it's available from the standard typing package and the way the type behaves across the supported Python versions is the same.

marcus-oscarsson commented 1 year ago

Sounds good :)

rhfogh commented 1 year ago

I tried to do some thorough type annotation on the files I was refactoring for the Qt GPhL GUI (see mxcubeqt PR #446, temporarily marked as WIP to gather your comments).

As an experiment I used inline type annotations rather than Google=-type doc strings. My first impression is that they are more compact, but maybe a bit more cluttered to read for the function definitions than teh Google-style doc strings(at least without a colour-coding editor).

Please could you let me know what you think about the style (both for function definitions and parameters). There seems to be several ways of doing this, and with your feedback I can rewrite the typing in a better style before making the final PR