Meson: Can we have a project wide consensus on how to import the typing module?

[dcbaker]

half of the code is structured like this:

import typing

def foo(arg1: str, arg2: typing.List[str]) -> typing.Optional[str]: ...

The other half like this:

from typing import(
   this, that, some other things, and, more stuff,
   and even more stuff,
)

def foo(art1: str, arg2: List[str]) -> Optional[str]: ...

Now, I really prefer the former. It avoids namspace pollution and minimizes git diffs because you don't have to worry about adding or removing things from the from typing import(...) lines. However, I'd really like to just have an agreement and convert all of the code to do the same thing so I can stop rebasing things and discover that a file has been changed from one form to the other in between.

[mensinda]

I personally prefer the from typing import ... style. My main point is that it makes the code more readable and keeps the function signatures (relatively) short:

class Foo:
    def bar(arg1: typing.List[int]. arg2: typing.List[typing.Dict[str, str]]) -> typing.Optional[typing.Union[int, typing.Dict[str, str]]]:
        pass

    def baz(arg1: List[int]. arg2: List[Dict[str, str]]) -> Optional[Union[int, Dict[str, str]]]:
        pass

Neither bar nor baz are short. However, I would always prefer baz over bar since it does not have the six redundant typing., which makes baz a lot more readable. Yes, having to remember that the from typing import might have to be updated is annoying, but (at least for me) a worthwhile tradeoff for the added readability.

Also, namespace pollution is not an issue, because all variables in meson are all lower case (flake8 should enforce that if I remember correctly). And someone adding classes like List, Dict, Optional, etc. shouldn't pass code review even if import typing is used.

[xclaesse]

@mensinda +1. I agree type annotation is useful, but it already makes function signature significantly harder to read, repeating 'typing' everywhere makes it even worse. Removing the namespace is a good compromise IMHO.

[dcbaker]

Also, namespace pollution is not an issue, because all variables in meson are all lower case (flake8 should enforce that if I remember correctly)

With my python developer hat on,

In python namespace pollution is always an issue because it's all public. Anything you import at the root module level that isn't a module or prefixed with _ is considered public API by convention, unless we're also going to maintain __all__ in every module to hide them? This isn't as much of a problem for us since we're building a program not a library, but it is going to lead to people import things through your module instead of from the source.

[xclaesse]

It should be catched by flake8, but the error message is not helpful:

xclaesse@xclaesse-x230:~$ cat test.py 
from typing import Dict


class Dict:
    pass


print(Dict())
xclaesse@xclaesse-x230:~$ flake8 test.py 
test.py:4:1: F811 redefinition of unused 'Dict' from line 1

It is also pretty easy to grep for "class Dict", etc, in the CI coding style checker.

[dcbaker]

Redefinition sure, but what about

a.py:

from typing import List

...

b.py:
```python
import A

def foo() -> a.List[str]:
```

[mensinda]

I think we are already maintaining __all__ in at least some modules.

In that case, we could also enforce this on a project wide scale.

[dcbaker]

Which would be even more maintenance burden to avoid typing 6 characters.

At this point if we could have a few more of the @mesonbuild/core-maintainers chime in I'll go with whatever the consensus is, even if I don't like the outcome.

[xclaesse]

At this point if we could have a few more of the @mesonbuild/core-maintainers chime in I'll go with whatever the consensus is, even if I don't like the outcome.

+1

[scivision]

I agree with either approach. A third approach: import typing as T or so.
In any case it would be good to use __all__.

Personally in my projects I use import typing except where there's long annotations (like Meson has) and I would use import typing as T. However, as long as __all__ is used, I don't mind from typing import

[jpakkane]

I like import typing because it gives you access to the entire thing, having to manually add and remove classes to the import command is tedious. import typing as T is also nice.

[amitdo]

The examples here encorage the from typing import ... style:
https://docs.python.org/3/library/typing.html

Same thing here:
http://mypy-lang.org/examples.html

[amitdo]

In CPython itself, from typing import ... is used in the tests.

[dcbaker]

People also tend to have using namespace std; in c++ examples, but that doesn't make it good practice. I'm also not sure tests are the best place to look for best practice. I often do things in tests I wouldn't do in production code for all kinds of reasons, mostly laziness.

I think we're basically split on the issue.

I would prefer the import typing as T solution to the from typing import, as it solves all of my concerns (no leaking, no need to maintain __all__, no need to maintain a list of import), but is much less verbose than typing; would that be an acceptable compromise to everyone?

[mensinda]

I personally still prefer from typing import but I could live with import typing as T.

However, I disagree with the argument that we don't have to maintain __all__ in that case. First of all, we are already doing it in ast, cmake and compilers. There are also other instances where stuff is imported via from X import Y, Z. So, we should also add __all__ to the other modules if we are really concerned about namespace pollution.

So, maintaining __all__ would be a good idea regardless of what import style we choose.

[mensinda]

Since no new arguments are being brought forth, can we go forward with changing the import style to import typing as T? This appears to be a good compromise between the two positions, and everyone seems to be happy with it.

[xclaesse]

(I still personally prefer no namespacing at all, but I have no strong opposition)