`Path` refactor

[etianen]

This PR implements the proposed refactor in #2944. As well as fixing the Python 3.13 problems, it adds some cool new things! 😎

The big change 💥

trio.Path now subclasses pathlib.PurePath!

This means all the sync method forwarding can be completely removed!

Since it now fits into the pathlib class hierarchy, it can be used interchangeably with other pure paths and methods that take pure paths. This also has the nice effect of making the wrapped async methods just work with regard to input types.

It's also now a non-virtual subclass of os.PathLike[str], which might help some static analysis tools...?

Two concrete subclasses PosixPath and WindowsPath also now exist, matching how pathlib does things. Instantiating a Path actually gets you the appropriate one for the platform, just like pathlib.

The medium change

Async method wrapping is now explicit, fixing Python 3.13. There are three high-level wrapper helpers that work for most cases (_wrap_method, _wrap_method_path and _wrap_method_path_iterable). For methods that don't fit into these, a low-level (_wraps_async) is used.

Other changes

Path is now slotted, to match pathlib. A minor speed boost, with luck!

Fixing Python 3.13

Unlike #2955, this fix actually works! Check it out:

================================================= test session starts ==================================================
platform darwin -- Python 3.13.0a3+, pytest-8.0.1, pluggy-1.4.0
rootdir: /Users/dave.hall/Workspace/trio
configfile: pyproject.toml
collected 33 items                                                                                                     

src/trio/_tests/test_path.py .................................                                                   [100%]

================================================== 33 passed in 0.05s ==================================================

My battle with pyright --verifytypes 😭

The pyright --verifytypes check does not like this syntax:

stat = _wrap_method(pathlib.Path.stat)

It claims this is an ambiguous type, asking me to annotate it with something that can't be expressed with the Python type system except for a per-method unweildy protocol!

The workaround would be to put in a dummy method and use a decorator syntax, which pyright thinks is absolutely okay despite inferring to exactly the same thing! It's also pretty disgusting to look at, and bloats the implementation with boilerplate.

@_wrap_method(pathlib.Path.stat)
def stat(self) -> Any:
    raise AssertionError("unreachable!")

I can't see any way to disable this check for just this file, or just these lines. Any suggestions?! 😭

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.64%. Comparing base (379d99b) to head (ba5b6c7).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #2959 +/- ## ========================================== - Coverage 99.64% 99.64% -0.01% ========================================== Files 117 117 Lines 17643 17594 -49 Branches 3176 3171 -5 ========================================== - Hits 17581 17532 -49 Misses 43 43 Partials 19 19 ``` | [Files](https://app.codecov.io/gh/python-trio/trio/pull/2959?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio) | Coverage Δ | | |---|---|---| | [src/trio/\_\_init\_\_.py](https://app.codecov.io/gh/python-trio/trio/pull/2959?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX19pbml0X18ucHk=) | `100.00% <100.00%> (ø)` | | | [src/trio/\_path.py](https://app.codecov.io/gh/python-trio/trio/pull/2959?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX3BhdGgucHk=) | `100.00% <100.00%> (ø)` | | | [src/trio/\_tests/test\_exports.py](https://app.codecov.io/gh/python-trio/trio/pull/2959?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX3Rlc3RzL3Rlc3RfZXhwb3J0cy5weQ==) | `99.61% <100.00%> (+<0.01%)` | :arrow_up: | | [src/trio/\_tests/test\_path.py](https://app.codecov.io/gh/python-trio/trio/pull/2959?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX3Rlc3RzL3Rlc3RfcGF0aC5weQ==) | `100.00% <100.00%> (ø)` | |

[jakkdl]

The pyright --verifytypes check does not like this syntax: stat = _wrap_method(pathlib.Path.stat) It claims this is an ambiguous type, asking me to annotate it with something that can't be expressed with the Python type system except for a per-method unweildy protocol! The workaround would be to put in a dummy method and use a decorator syntax, which pyright thinks is absolutely okay despite inferring to exactly the same thing! It's also pretty disgusting to look at, and bloats the implementation with boilerplate.

The error message is specifically about it being ambigious and could be inferred differently by other type checkers, so whether it's being inferred to the same thing by pyright is not really relevant. In most other cases we've resorted to doing the disgusting/bloated boilerplate to get around these errors, but I'll have a closer look later on whether I think it's worth it. We are after all running type checks on both mypy & pyright, so we can probably ignore it.

I can't see any way to disable this check for just this file, or just these lines. Any suggestions?! 😭

Maybe I should resuscitate https://github.com/python-trio/trio/pull/2910 and get the output-filtering script finished/merged.

[etianen]

The error message is specifically about it being ambiguous and could be inferred differently by other type checkers

I think I'm disagreeing here with pyright. I can write the same thing using a decorator syntax, which provides no additional typing information, and it's happy. It just seems to be complaining that I'm doing a variable assignment and not explicitly annotating the variable.

This is related to my point about the static visibility linter, really. This check seems to have a very uncompromising idea of what's ambiguous. The use of ParamSpec and Concatenate here has, AFAIK, absolutely only one possible meaning. That's not ambiguous! It's simply not explicit.

I'm a huge fan of static analysis tools in general, and favor very strict typing and linting. But to me, these checks feel overboard. I realize that this exactly the argument people make to me about strict mypy checks, so maybe it's a case of comfort levels? But at the point inheritance and strictly-typed metaprograming become forbidden, it feels too much to me.

I'm left in a bit of a quandry for this PR though. Hopefully you have a solution in mind with #2910.

[etianen]

Also, apologies if the tone of my messages comes over as overly critical! I'm a big fan of how welcoming this project is to new contributors, so this stems from a concern about placing too-high barriers in front of new contributions.

It's hard to express frustration without sounding... frustrated. 🥵

[jakkdl]

disagreeing with pyright seems very fair in this case, and given how complex the Path wrapping is already I'm open to just ignoring what --verifytypes says. I don't know for sure why it forbids assignment here. Will look at stuff tomorrow :)

[etianen]

Thanks for the feedback! I've implemented the suggested changes. The new docs in particular look very nice! 💪

I'll await your findings on the --verifytypes problem. I couldn't find a way to selectively ignore this file, so hopefully you'll have a better time of it than me! 😅

[CoolCat467]

One slightly interesting point https://trio--2959.org.readthedocs.build/en/2959/reference-io.html#trio.Path.link_to

[etianen]

One slightly interesting point https://trio--2959.org.readthedocs.build/en/2959/reference-io.html#trio.Path.link_to

I'm assuming these docs were built not on Python 3.12? The method is indeed gone in 3.12.

[A5rocks]

Yeah we build docs on an older Python. No matter what method availability depends on Python version and we don't have a setup for documenting that. I think I had a PR open to build docs on 3.12 but closed it when I realized that it would mean our Path docs would be less useful for anyone not using 3.12 (due to method removals).

[etianen]

Thanks for the feedback @A5rocks ! 🙇

I'm seeing two outstanding issues here, both related to the trio linting requirements:

1. trio.Path cannot be made final
2. trio.Path cannot be made unambiguous according to pyright without adding a bunch of boilerplate.

Hopefully excluding trio.Path from the checks for (1) is not a blocker. 🙏

Hopefully there is a way to exclude trio.Path from the checks for (2), since excluding it is the right answer here. The types are not ambiguous AFAIK.

In terms of it being a slightly funny API, I do agree, but it's the funny API from pathlib. 🤷 And if the purpose of trio.Path is to be an an async pathlib.Path, then being able to interop with and behave like the stdlib is pretty neat, IMO.

[jakkdl]

Spent all day doing other stuff instead of doing #2910, but if the verifytypes test becomes the only blocker we can ignore it and/or temporarily make it not fail CI if it fails, and I'll fix it soon :tm:

[etianen]

I've disabled the requirement for trio.Path to be final. Hopefully that's fine. 🙏

As a bonus, I've added explicit support for subclassing trio.Path, making this a feature! @TeamSpen210 mentioned subclassing in this comment. Well, now people can subclass trio.Path to wrap pathlib.Path subclasses.

As a bonus, this speeds up async methods slightly by not performing an os.platform check ever time, since trio.WindowsPath and trio.PosixPath explicitly reference pathlib.WindowsPath and pathlib.PosixPath

[etianen]

So I think the only blocker here is --verifytypes. There's still a few open conversations in the air about minor issues, so it would be good to get some other opinions to get them resolved.

[TeamSpen210]

In regards to Pyright's checks, the intent I think is to warn about any public API that would require use of type inference to determine the type, meaning that it's possible that a less powerful/intelligent checker might give different results. In this case we deliberately do want to use inference to prevent having to manually write out the signatures, so ignoring it would be fine.

[etianen]

@TeamSpen210 100% The only thing preventing me is I can't find any way to selectively disable it just for this file!

[etianen]

I've just had another look at this. Unfortunately, verifytypes does not respect:

• exclude
• ignore
• defineConstant

The only solution I can see is the solution in #2910. Do we keep this open and wait for that to merge, or do we temporarily disable the check?

[A5rocks]

The only solution I can see is the solution in https://github.com/python-trio/trio/pull/2910. Do we keep this open and wait for that to merge, or do we temporarily disable the check?

I'm sure specifically filtering pyright errors should be a pretty simple thing to do; let's wait a few days and see if that happens. If it doesn't, then temporarily disabling the check sounds good. (I'm hoping we can get a release out in ~a week, so that this and a bunch of changes can go out)

[TeamSpen210]

Perhaps we should open an issue for Pyright. Thinking about it, it seems like a bug that verifytypes doesn't pick up on inference being required for the decorator, that's inconsistent.

[etianen]

Hopefully that's all points addressed! Some directly, others laterally.

[A5rocks]

pyright changes are merged, you should be able to python src/trio/_tests/check_type_completeness.py --overwrite-file (or something like that) and that should be it! i tried doing that but for some reason github codespaces doesn't recognize that i can push to the branch and i dont want to bother setting this up locally.

[jakkdl]

I added some logic to ignore everything instead of listing all the errors in the json. I feel like has_docstring_at_runtime should've been able to pick up the docstrings, but I haven't bothered figuring out why it doesn't.

[etianen]

It's in! Thanks all!

[jakkdl]

Thank you! And sorry for the delay with finishing up #2910 :)

[A5rocks]

fyi this went out in 0.25.0!