Static tools doesn't see class members (at least for trio.Path)

[jakkdl]

Jedi:

> import jedi
> script = jedi.Script("import trio; trio.Path.")
> comp = script.complete()
> [cc.name for cc in comp if not cc.name.startswith('_')]
Out: ['mro', 'open']

mypy also doesn't see them, which is probably the bigger problem

$ mypy trio/tests/test_path.py
...
trio/tests/test_path.py:189: error: "Type[Path]" has no attribute "joinpath"  [attr-defined]

which means downstream users will get typing errors if they use trio.Path in their code.

I haven't figured out how to check pylint, but otherwise it should be doable to modify test_static_tool_sees_all_symbols to also check class members.

For trio.Path, there's presumably some reason it's doing lots of magic instead of inheriting from Path - so if that's not on the table one would either have to define all the methods, or if that's what _forward_magic is for it's maybe possible to list them there, or some other constant.

[Fuyukai]

The magic with Path is one of the less egregious dynamic-style things in the lib because you actually cannot inherit from Path:

>>> from pathlib import Path
>>> class MyPath(Path):
...     pass
...
>>> p = MyPath("/tmp")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.10/pathlib.py", line 960, in __new__
    self = cls._from_parts(args)
  File "/usr/lib/python3.10/pathlib.py", line 594, in _from_parts
    drv, root, parts = self._parse_args(args)
  File "/usr/lib/python3.10/pathlib.py", line 587, in _parse_args
    return cls._flavour.parse_parts(parts)
AttributeError: type object 'MyPath' has no attribute '_flavour'

(that being said, I still do not like trio.Path)

[TeamSpen210]

Pathlib will actually be subclassable, as of 3.12. Before then you'd have to set the _flavour attribute to the private _PosixFlavour() or _WindowsFlavour() classes.

For statically defining the methods, that could be done for mypy by using if typing.TYPE_CHECKING:, then defining stub methods for them. Or use a .pyi file. Looks like jedi will support that too.

[jakkdl]

Pathlib will actually be subclassable, as of python/cpython#31691. Before then you'd have to set the _flavour attribute to the private _PosixFlavour() or _WindowsFlavour() classes.

Hm, might be worth rewriting trio.Path with subclassing then.

For statically defining the methods, that could be done for mypy by using if typing.TYPE_CHECKING:, then defining stub methods for them. Or use a .pyi file. Looks like jedi will support that too.

I wasn't sure if jedi would check typing.TYPE_CHECKING - but as it does that makes it a simple fix.

I wrote a test, and other than trio.Path, and if you exclude mro (which jedi says all exceptions have, but neither dir nor inspect.getmembers do), and dunders*, there's only really a few stragglers:

Missing:
{'trio.socket.gaierror': {'characters_written'},
 'trio.socket.herror': {'characters_written'}}

Extra:
{'trio.StapledStream': {'send_stream', 'receive_stream'}}

* The dunders are a mess and idk how much of it is stuff that's a problem / fixable / hallucinated vs hidden by jedi/dir/inspect.getmembers or similar. So I'd personally lean towards ignoring them for now and adressing that if/when anybody encounters an error in practice.

[jakkdl]

gaierror and herror are re-exported from trio.socket, so idk why they differ - but can probably be ignored.

I have no idea why dir doesn't see send_stream and receive_stream in StapledStream - when it can see attr.ib() members on other classes plenty fine.

[smurfix]

if you exclude mro

Well, __mro__ is special. It doesn't show up in a type's __dir__ but you can still access it. There are a handful of other attributes like that, e.g. __base__.

[A5rocks]

Hm, might be worth rewriting trio.Path with subclassing then.

Typecheckers won't like that, FWIW. (Cause subclassing by overriding methods with async methods).

We already have https://github.com/python-trio/trio/blob/4f17d2b2693d961f2c12df76a5ce4f80ec3f3178/trio/_path.pyi, maybe it's time to fill that! I don't know about pylint, but mypy, pyright, and (supposedly) jedi will support us if we just add some stubs in there. Maybe.

A better solution IMO would be adding stub methods on the class itself (with proper types but that just raise NotImplementedError or something) where we could maybe get rid of some of this boilerplate: https://github.com/python-trio/trio/blob/4f17d2b2693d961f2c12df76a5ce4f80ec3f3178/docs/source/reference-io.rst#asynchronous-file-objects and generate it from the class members themselves.

[jakkdl]

if you exclude mro

Well, __mro__ is special. It doesn't show up in a type's __dir__ but you can still access it. There are a handful of other attributes like that, e.g. __base__.

ooh thanks, this lead me to https://stackoverflow.com/a/18035950/15544350 which resolved my confusion

[jakkdl]

Hm, might be worth rewriting trio.Path with subclassing then.

Typecheckers won't like that, FWIW. (Cause subclassing by overriding methods with async methods).

Oh yeah, derp

We already have https://github.com/python-trio/trio/blob/4f17d2b2693d961f2c12df76a5ce4f80ec3f3178/trio/_path.pyi, maybe it's time to fill that! I don't know about pylint, but mypy, pyright, and (supposedly) jedi will support us if we just add some stubs in there. Maybe.

Oh I'd completely forgotten about the stub file - I was banging my head against the wall the other day because mypy didn't see my updates to _path.py >.>

A better solution IMO would be adding stub methods on the class itself (with proper types but that just raise NotImplementedError or something) where we could maybe get rid of some of this boilerplate: https://github.com/python-trio/trio/blob/4f17d2b2693d961f2c12df76a5ce4f80ec3f3178/docs/source/reference-io.rst#asynchronous-file-objects and generate it from the class members themselves.

It would be nice not to have to write out the full stub of 50 functions, and be able to do it with some wrapper/decorator that mypy+docgen+jedi accepts. So one can write

class Path:
  [...]
  is_socket = make_async(pathlib.Path.make_async)
  with_suffix = make_async(pathlib.Path.with_suffix)
  [...]

Although actually, at that point it might just be possible to get rid of __getattr__ and the other magic entirely.

[TeamSpen210]

I don't think it'll work for Jedi (davidhalter/jedi#1812), but you can do that wrapper via ParamSpec:

from typing import TypeVar, ParamSpec, Awaitable
ArgsT = ParamSpec('ArgsT')
RetT = TypeVar('RetT')
def make_async(func: Callable[ArgsT, RetT) -> Callable[ArgsT, Awaitable[RetT]]: ...

[jakkdl]

I don't think it'll work for Jedi (davidhalter/jedi#1812), but you can do that wrapper via ParamSpec:

from typing import TypeVar, ParamSpec, Awaitable
ArgsT = ParamSpec('ArgsT')
RetT = TypeVar('RetT')
def make_async(func: Callable[ArgsT, RetT) -> Callable[ArgsT, Awaitable[RetT]]: ...

turns out testing things is a good idea ... this doesn't seem to work?

error: Invalid self argument "Path" to attribute function "group" with type "Callable[[Path], Awaitable[str]]"  [misc]

[TeamSpen210]

Ahh, mypy is picking up on the self argument not matching. Should be resolvable via Concatenate though.

def make_async(func: Callable[Concatenate[pathlib.Path, ArgsT], RetT]) -> Callable[Concatenate[trio.Path, ArgsT], Awaitable[RetT]]: ...

[jakkdl]

Ahh, mypy is picking up on the self argument not matching. Should be resolvable via Concatenate though.

def make_async(func: Callable[Concatenate[pathlib.Path, ArgsT], RetT]) -> Callable[Concatenate[trio.Path, ArgsT], Awaitable[RetT]]: ...

wow now we're getting into real dark typing magic, alas still no go:

trio/_path.py:211: error: Argument 1 to "_make_async" has incompatible type "Callable[[], Path]"; expected "Callable[[Path], Path]"  [arg-type]

...

trio/_path.py:245: error: Argument 1 to "_make_async" has incompatible type "Callable[[Self, str], Self]"; expected "Callable[[Path, str], Self]"  [arg-type]
trio/_path.py:245: note: This is likely because "with_suffix of PurePath" has named arguments: "self". Consider marking them positional-only

loads of the latter, just a few of the former

[jakkdl]

oh, it also fails with

trio/_path.py:23: error: The first argument to Callable must be a list of types, parameter specification, or "..."  [valid-type]
trio/_path.py:23: note: See https://mypy.readthedocs.io/en/stable/kinds_of_types.html#callable-types-and-lambdas

at the definition 🙃

edit: wait no, stopped getting that error for some reason