Add stubs for wrapped methods to trio.Path, add tests for mypy seeing exported symbols, and test for seeing class members

[jakkdl]

For #2630

[codecov]

Codecov Report

Merging #2631 (c6ac460) into master (1a7a9c5) will decrease coverage by 1.59%.
The diff coverage is 62.50%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2631      +/-   ##
==========================================
- Coverage   92.43%   90.84%   -1.59%     
==========================================
  Files         118      118              
  Lines       16352    16387      +35     
  Branches     2945     1309    -1636     
==========================================
- Hits        15115    14887     -228     
- Misses       1124     1358     +234     
- Partials      113      142      +29     

Impacted Files	Coverage Δ
trio/tests/test_exports.py	75.51% <57.64%> (-21.68%)	⬇️
trio/_path.py	100.00% <100.00%> (ø)

... and 47 files with indirect coverage changes

[TeamSpen210]

Actually, does trio want a dependency on typing_extensions? #2507 suggests not, so it'd be better to move the import into the if TYPE_CHECKING guard, then just unconditionally import from typing.

[jakkdl]

Actually, does trio want a dependency on typing_extensions? #2507 suggests not, so it'd be better to move the import into the if TYPE_CHECKING guard, then just unconditionally import from typing.

ah yes, good catch

[jakkdl]

I should maybe spin up a virtual machine at some point so I can test windows stuff better lol

[jakkdl]

Huh, looks like pypy has different dunders? I'm guessing they can just be ignored.

  'trio.EndOfChannel': {'extra': {'__basicsize__',
                                 '__dictoffset__',
                                 '__itemsize__',
                                 '__sizeof__',
                                 '__weakrefoffset__'},
                       'missing': {'__unicode__'}},

Windows ones are failing due to path having a colon in C: which is easily worked around.

[TeamSpen210]

Yeah those aren't relevant. __unicode__ would be Py2 remnants, PyPy mustn't have removed that yet. The rest are how the memory layout of the object is defined/exposed for introspection, which isn't something Python code should care about unless it's using ctypes or something.

[Fuyukai]

Truly incredible. Now we have the worst of both worlds: you have to type out all the wrapper declarations manually and you're testing that you even wrote out the wrapper declarations - and if we ever want to have real asynchronous file I/O all of this has to go anyway.

[A5rocks]

I think it's worth the effort to provide better autocomplete! (Sorry if I'm misinterpreting your tone!)

The test is just a continuation of what we already test -- just that we are checking that class members match and not just module members.

[jakkdl]

Truly incredible. Now we have the worst of both worlds: you have to type out all the wrapper declarations manually and you're testing that you even wrote out the wrapper declarations - and if we ever want to have real asynchronous file I/O all of this has to go anyway.

I could've implemented the class member check as making the export test more thorough, but was easier to just write a new one than modifying the old one to also do class members. So yes the tests are very much what's currently done already, except with one more tool and testing the public class methods that weren't.

_path.py is messy and ugly, I don't disagree with that, but it's the only class that requires this much of a mess, and it's not like the interface of pathlib.Path is gonna change anytime soon, so this shouldn't introduce any maintenance overhead or anything.

And it's not like I came up with this for nothing - I ran mypy on the current tests (cause typechecking the tests is a good way to get free tests for whether the types on trio is correct) and it failed whenever it did anything with trio.Path because the methods were invisible to it. So I fixed that, and wrote a test to see if any other classes were broken. /shrug

And I have absolutely no problem with all of this getting tossed out when systems are improved!

[jakkdl]

These last errors are confusing me (and then there's other problems currently untested that'll need adressing), but I think it boils down to dir(pathlib.Path) includes 'group', 'owner' and 'is_mount' on both win32 and not - but if you'd try calling them on win32 you'd get an error. And mypy solves that by saying they're not attributes of pathlib.Path if run with --platform=win32.
I thought that maybe dir(trio.Path) was bugged on win32 in listing them, but if pathlib.Path also does it then I guess it isn't really a bug - it's just mypy special-casing it and so the correct thing is to expect the current status quo with dir() and mypy not being the same, and special-case them in the tests.

[jakkdl]

Okay since getting the stubs correct for the path methods wasn't trivial, I'll postpone that to a different PR so this one can get merged.

[jakkdl]

would somebody mind reviewing this? It's ready for merge unless there's any more comments.

[A5rocks]

I can check this out in a bit, also the new 3.12 dev errors are concerning (they're not cause you) and we gotta fix those!

[A5rocks]

Sorry, haven't gotten to this! Eventually I will, don't worry.

[A5rocks]

(just updating this branch based on master as that (should) fix the macos pypy-3.8-nightly cancellation and the formatting check)

[A5rocks]

I was telling myself not to nitpick too much but... Oops. If something feels like a nitpick, feel free to ignore it! (or if it's missing something too obvious, I guess)

[A5rocks]

It would be nice to be able to auto-generate this (read from relevant file in https://github.com/python/typeshed and change the types... like just stick an async in the right places and call it a day). I suppose that's all part of "how will we do this" that is the followup, so ignore this :^)

[A5rocks]

This line isn't the one I wanted to comment on, but I can't comment on it cause it's out of the diff github shows :(

Namely, the line that is os.PathLike.register(Path). Presumably we could subclass os.PathLike and that would help some typing stuff? I don't know though.

[TeamSpen210]

Definitely, register isn't going to be fully supported everywhere. It'd also be better to inherit to indicate that we provide string paths, not bytes. A little tricky though since it's only as of 3.9 that you could subscript PathLike:

if TYPE_CHECKING:
    _StrPathLike = os.PathLike[str]
else:
    _StrPathLike = os.PathLike
...
class Path(_StrPathLike):

[jakkdl]

let's leave that to a different PR that fully types trio.Path, can either continue with that in #2630 or open a different issue

[jakkdl]

Fixed all changes requested, except where noted. Good nitpicks @A5rocks! ❤️

[A5rocks]

Seems great from a cursory glance, we can always make a bunch of followups!

[jakkdl]

Okay I'm done fighting with codecov, merging!