Add stubs for typing

[blueyed]

This is meant to be used for pytest, which mainly uses py.path.local, and is very much work in progress.

Continuation of https://github.com/pytest-dev/py/pull/231

[blueyed]

Will be nice to squash to one commit.

Sure. Would be nice to do it via GitHub's UI, but it's disabled here also. I think keeping the history here is useful in general, so we should use yet another PR then for the squashed result (or merge it manually).

As for "WIP": I think it's good to use it via this/your branch for a while before merging.

[bluetech]

As for "WIP": I think it's good to use it via this/your branch for a while before merging.

Do you mean use it in pytest or something else? If pytest, then there is not much using the stubs beyond running mypy on the code with the stubs installed, and we did do that.

Are there any other projects using py + mypy that you know about? I can check them as well.

[blueyed]

As for "WIP": I think it's good to use it via this/your branch for a while before merging.

Do you mean use it in pytest or something else? If pytest, then there is not much using the stubs beyond running mypy on the code with the stubs installed, and we did do that.

Yes. But I do not think we need to merge it already, and certainly not until others agree that it should be e.g. shipped. Having the branch here is good enough for us to help with typing pytest, and could even be used on our CI already via it - so there is no need to rush it.

Are there any other projects using py + mypy that you know about? I can check them as well.

I don't know of any.

[bluetech]

Yes. But I do not think we need to merge it already, and certainly not until others agree that it should be e.g. shipped.

I suspect that with a WIP label it won't get attention by others. I think the stubs are complete, and there is not much to test beyond just using it once in pytest, so I just wonder what more you think is needed in order to get the WIP label down.

[blueyed]

@bluetech agreed. Not really WIP, but waiting for feedback and should not get merged as-is. I've removed the label.

[bluetech]

@blueyed We should add this diff:

diff --git a/py/path.pyi b/py/path.pyi
index acbee979..2556a732 100644
--- a/py/path.pyi
+++ b/py/path.pyi
@@ -49,7 +49,7 @@ if sys.version_info >= (3, 6):
 else:
     class _PathLike(Generic[AnyStr]):
         def __fspath__(self) -> AnyStr: ...
-_PathType = Union[bytes, Text, _PathLike[Any]]
+_PathType = Union[bytes, Text, _PathLike[Any], local]

 class local:
     class ImportMismatchError(ImportError): ...

While I thought py.path.local would just be covered by the os.PathLike, it's not the case currently (https://github.com/python/typeshed/issues/2808).

[blueyed]

@bluetech ok, please push it here then - I cannot really tell/help in that regard.

[bluetech]

Huh, didn't notice this PR is now based on my branch.

I pushed this and another fix.

[blueyed]

@bluetech please git push bluetech 1.8.1 (i.e. the latest tag to your repo), so that setuptools-scm gets the correct/proper version.

[blueyed]

You might also want to fix the conflict with appveyor.yml, so that CI runs. Could/should also revert https://github.com/pytest-dev/py/pull/232/commits/66fc1bf4d7f5cebb959f98cbd282f33cbe2dda61 then - I'll go ahead and do that.

[blueyed]

@bluetech re the above: I do not have permission to push to this branch also (allow it via checkbox on the right?). But would still not allow me to push tags probably.

[bluetech]

@blueyed done.

Do you think there is anything else blocking this PR? Should we ask anybody for additional review or +1s? py.path.local shows up everywhere in pytest, so it'd be really nice to have it :)

[blueyed]

Do you think there is anything else blocking this PR?

Not really, apart from that we/you are still added fixes/improvements to it.

Should we ask anybody for additional review or +1s?

Sure.

py.path.local shows up everywhere in pytest, so it'd be really nice to have it :)

I'm going to add this branch to the "mypy" tox env, which then can be run on CI already. Having the version (tag) for it was a requirement.

[bluetech]

Okay, pinging some names I recognize from the contributor's list.

@nicoddemus @RonnyPfannschmidt @asottile

This PR adds type stubs to py. The stubs only cover the modules that pytest uses, but are comprehensive and strict, otherwise. We tested pytest's typing with typed py installed and it works well. The py.typed file indicates the stubs are "partial", so projects which use modules which we didn't cover shouldn't be badly affected.

Would be great to have your +1's, unless you think this is not a good idea.

[asottile]

the stubs seem fine, though for a library which is "maintenance only" I think this extends a bit beyond that -- that said it does improve pytest's ability to typecheck itself so I'm a little torn 🤔

[blueyed]

@bluetech you have not pushed the latest tag, have you? Please do so.

[bluetech]

@bluetech you have not pushed the latest tag, have you? Please do so.

Pushed. (BTW, I don't see the "allow edit from maintainers" checkbox).

[blueyed]

@bluetech Thanks.

(BTW, I don't see the "allow edit from maintainers" checkbox).

Likely because I've created the PR then..

[blueyed]

commit cae97e28 (HEAD -> pyi)
Author: Daniel Hahler <git@thequod.de>
Date:   Tue Jan 28 22:14:25 2020 +0100

    path: __truediv__, __div__, join: returns local

diff --git a/py/path.pyi b/py/path.pyi
index b7879c3e..e803ada2 100644
--- a/py/path.pyi
+++ b/py/path.pyi
@@ -65,8 +65,8 @@ class local:
     def __gt__(self, other: object) -> bool: ...
     def __add__(self, other: object) -> local: ...
     def __cmp__(self, other: object) -> int: ...
-    def __div__(self, other: _PathType): ...
-    def __truediv__(self, other: _PathType): ...
+    def __div__(self, other: _PathType) -> local: ...
+    def __truediv__(self, other: _PathType) -> local: ...
     def __fspath__(self) -> str: ...

     @classmethod
@@ -132,7 +132,7 @@ class local:
     def isdir(self) -> bool: ...
     def isfile(self) -> bool: ...
     def islink(self) -> bool: ...
-    def join(self, *args: _PathType, abs: int = ...) -> Any: ...
+    def join(self, *args: _PathType, abs: int = ...) -> local: ...
     def listdir(
         self,
         fil: Optional[Union[str, Text, Callable[[local], bool]]] = ...,

[codecov-io]

Codecov Report

Merging #232 into master will not change coverage by %. The diff coverage is 66.66%.

@@           Coverage Diff           @@
##           master     #232   +/-   ##
=======================================
  Coverage   82.06%   82.06%           
=======================================
  Files          55       55           
  Lines       10161    10161           
  Branches     1141     1141           
=======================================
  Hits         8339     8339           
  Misses       1558     1558           
  Partials      264      264           

[blueyed]

@bluetech seems you've not really applied the patch? (2cf11b7 is empty) You can cherry-pick it via cae97e28.

I guess you could also invite me as collaborator to your fork.

[bluetech]

@blueyed Oops. Applied now. I also went and squashed it. I know you asked not to, but it was pretty terrible! I put the old history here: https://github.com/bluetech/py/tree/pyi-old

[bluetech]

@asottile

the stubs seem fine, though for a library which is "maintenance only" I think this extends a bit beyond that -- that said it does improve pytest's ability to typecheck itself so I'm a little torn

Since this doesn't make any code changes, I think the risk is quite low. But I understand the hesitation.

To move this forward, maybe we can submit it to typeshed instead. @blueyed, WDYT?

[blueyed]

@bluetech From my point of view it works for me by using this branch locally and for CI. I do not think it is "worth" adding to typeshed, given that only pytest uses it probably. I'm not opposed to merging and properly releasing it though, of course - but cannot really tell if that implies going through extra things when updating/fixing them.

[bluetech]

I see 3 options to proceed:

1. Use py from a branch. This doesn't work for users, or official pytest CI. I think it's better to avoid this, to allow everyone to benefit.
2. Merge this PR and release py. This is the best outcome IMO, but seems to have maintenance concerns and py is officially frozen. So probably not going to happen?
3. Add to typeshed. I can manage and maintain it myself. Doesn't incur a maintenance burden on py itself. Typeshed is open to 3rd party stubs.

That's why going through typeshed seems like the best course to me for now. But of course I wouldn't submit it without consensus.

[RonnyPfannschmidt]

im ok with option 2 in a feature release - this is clearly a help for any downstream that wants to adopt typing

[bluetech]

Added a few improvements to py.xml while adding annotations to _pytest.junitxml.

[bluetech]

... and squashed again.

@RonnyPfannschmidt said:

im ok with option 2 in a feature release - this is clearly a help for any downstream that wants to adopt typing

Thanks! So I guess we have +1 from @blueyed, @RonnyPfannschmidt (and me), and a -0 (?) from @asottile.

If anyone with permissions feels like merging this (and maybe releasing), that would be great.

If there will be any issues reported after releasing this, I'll make sure to handle them (I subscribed to issues on this repo).

[bluetech]

There were a couple of omissions in the py.xml types, I fixed them:

• Didn't accept py.xml.raw where it's allowed
• Didn't accept Iterable of Tags where it's allowed

[bluetech]

OK, I made a few updates:

• Added CHANGELOG entry (marked for a 1.9.0)

• Somewhat belatedly I understood that partial in py.typed doesn't do what I thought it does, so I removed it and added explicit Anys for all of the types & modules we didn't cover.

• Made py.path.local explicitly inherit from os.PathLike because it is actually not defined as a Protocol in typeshed. This fixes pathlib.Path(py.path.local()) (at least on Python>=3.6).

Finally, pytest master is currently completely clean with this PR.

So I think this is ready.

[nicoddemus]

Awesome! Let's merge and release this then!