Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contributed stubs for "packaging" package #4427

Closed
wants to merge 5 commits into from

Conversation

erictraut
Copy link
Contributor

No description provided.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

packaging seems to ship with inline stubs (edit: inline types), https://github.com/pypa/packaging/blob/master/packaging/py.typed

@erictraut
Copy link
Contributor Author

It has a py.typed file, but I don't see any stub files.

@erictraut
Copy link
Contributor Author

@hauntsaninja, you might be curious about why I'm submitting all of these new stubs. As you're probably aware, Microsoft recently released Pylance, the Python language server built on top of Pyright. It's still in "preview", but we already have about 85K developers who have opted in to the preview.

We've gathered some stats about which popular packages are missing type stubs. Missing stubs lead to a less-than-great experience for developers because completion suggestions are inaccurate or incomplete. We're using our data to prioritize our contributions of stubs to fill in the gaps. We'd love to see all of the most-frequently-used packages have either inlined stubs or stubs within typeshed!

We recently held a day-long internal "stub-a-thon" event to encourage Microsoft employees to help generate type stubs. The stubs I've been submitting are the result of this effort.

@hauntsaninja
Copy link
Collaborator

We're very grateful for the contribution of these stubs to typeshed! It's especially exciting that these contributions align well with the stubs users most need. I'm also appreciative of your spree of correctness and completion PRs previous to this.

Let me know if there's anything I can do to make it easier to contribute stubs en masse. For instance, if you're okay with other people pushing to your branches, other contributors could help fix nits and get CI green.

In the meantime, my colleagues and I have been enjoying Pylance, so thank you very much for its existence!

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Aug 9, 2020

Regarding this PR, packaging doesn’t have stub files, but it does have inline stubs (edit: inline types). I’m not the most familiar with PEP 561, but I believe even if we were to merge this, typeshed’s stubs would be ignored by type checkers in favour of the inline stubs (edit: inline types).

@srittau
Copy link
Collaborator

srittau commented Aug 9, 2020

It's really great to see all these stubs. This was already mentioned in one of the other PRs, but I would also suggest just to put these stubs into third_party/3 instead of 2and3, unless the stubs specifically were written with Python 2 in mind. Python 2 support is always a bit tricky, and can easily lead to errors. If someone wants to put the effort in to make a particular stub "Python-2-safe", we can always move the stub later.

@gvanrossum
Copy link
Member

Agreed it’s great to see these new stubs appearing.

In the past we’ve asked package authors for permission. Maybe we should still do that? In particular the authors of ‘packaging’ might have an opinion, since they have a pay.typed flag file.

@erictraut
Copy link
Contributor Author

Thanks for all the help, support and encouragement. It's great to see the community rally around this for the benefit of all Python developers.

Maybe I'm using the terminology incorrectly, but I have been using the term "inline stubs" to mean "pyi files that ship within a package alongside the py files, as opposed to a separate stub package". And I've been using the term "inline types" to mean "type annotations included within the py files". Am I using these terms correctly? PEP 561 doesn't seem to formally differentiate between these two cases. It defines "inline" and "stubs", but it doesn't define the term "inline stubs".

"inline" - the types are part of the runtime code using PEP 526 and PEP 3107 syntax (the filename ends in .py).
"stubs" - files containing only type information, empty of runtime code (the filename ends in .pyi).

I think it's important to distinguish between "stub files that ship with a package" and "annotated py files" because annotated py files will never be as complete as stub files when describing the official interface contract for a library — at least not without changes to the Python spec regarding symbol hiding and re-exports. Also, runtime code typically cannot take advantage of newer type features while still maintaining compatibility with older versions of Python, so even when annotations are "complete", they are invariably lower quality than what could be provided with a stub file. Plus, annotations within implementations are rarely complete, and unlike with stubs, it's difficult to verify their completeness.

For all of those reason, we would like to see a push for all library authors to ship stub (pyi) files in some form or another — either within their package, as a separate stub package, or on typeshed. Additional tooling may be required to help library authors create and maintain such stub files.

Incidentally, Pylance currently pays no attention to the py.typed file. If it finds pyi files in the package (or anywhere in the configured search paths), it uses them. If it doesn't find pyi files, it does its best to extract as much type information as it can from the py files it finds (using type annotations where provided and type inference otherwise).

Perhaps the typing-sig would be a better place for this discussion, but if the typeshed maintainers have any thoughts on the above, I'd be interested in them.

@gvanrossum
Copy link
Member

gvanrossum commented Aug 9, 2020

Maybe I'm using the terminology incorrectly, but I have been using the term "inline stubs" to mean "pyi files that ship within a package alongside the py files, as opposed to a separate stub package". And I've been using the term "inline types" to mean "type annotations included within the py files". Am I using these terms correctly? PEP 561 doesn't seem to formally differentiate between these two cases. It defines "inline" and "stubs", but it doesn't define the term "inline stubs".

I think using "inline stubs" that way is confusing; let me propose "in-package stubs", which is clear. ("Stub" always refers to a .pyi file.)

I think pylance's approach to extracting as much type info as possible from installed packages is fine, but your users will be the final arbiters. Whether to encourage package authors to include separate stubs even if they already have inline types, I'm not sure -- the export semantics may be unfortunate but in my experience they rarely cause trouble -- nobody writes from tempfile import os. And package authors appreciate the checking of their own code that happens with inline types. (Does pyright check a package against its own stubs if both .py and .pyi files are present?)

I do think it's better to bring all this up on typing-sig, you'll reach more people than here.

@hauntsaninja
Copy link
Collaborator

Sorry for my confusing diction, agree that "inline stubs" should be banished to wherever it is words go when they have no clear meaning; "in-package stubs" and "inline types" are far clearer.

My sense was that many would like to see the use of inline types over stubs. As a maintainer of typeshed, I can definitely say that keeping stubs up to date takes some work; if I were a library author, maintaining an __all__ and a couple if TYPE_CHECKING branches to use newer type features would seem easier than maintaining in-package stubs. Anyway, I'll save it for typing-sig :-)

@erictraut
Copy link
Contributor Author

What's the conclusion on this one? If typeshed isn't willing to accept this stub, we will probably ship it separately with Pylance/Pyright.

@gvanrossum
Copy link
Member

What's the conclusion on this one? If typeshed isn't willing to accept this stub, we will probably ship it separately with Pylance/Pyright.

(a) Why can't pylance use the stubs shipped with the package itself?

(b) Given that the package itself ships with stubs, why don't we ask the project whether they are okay with having another set of stubs in typeshed? I worry about the redundancy and about divergence between the two sets. (We used to require permission from third party packages; we don't do that any more, although I can't recall the discussion about it -- it may have happened while I was tuned out. @JelleZijlstra Do you recall?)

@erictraut
Copy link
Contributor Author

This package doesn't ship stubs. It provides only partial (incomplete) type information specified via python-2 type comments, which are not part of the Python spec.

If the authors of this library would like to incorporate the contributed stubs as part of their package, that would be a fine solution.

@erictraut
Copy link
Contributor Author

@brettcannon, are you a maintainer of this library? Any thoughts on this?

@hauntsaninja
Copy link
Collaborator

Re: permission from third party packages, it was changed to a notification: #3443, python/peps#1218 These refer to some discussion on typing-sig
If Brett doesn't respond or isn't a maintainer, the next step seems to be to open an issue on packaging

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wherever they end up, these stubs are going to be useful, so I reviewed them against the implementation and added some comments.


class Op(Node):
def serialize(self) -> str: ...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a number of other objects like VERSION_CMP at runtime. They're not explicitly private, but I don't know if they're part of the intended API of the package. They're not in __all__ but neither are Variable, Value, and Op.

requirements.py has a similar set of all-caps pyparsing-based names. They look like implementation details to me but maybe we'll have to add them to the stub at some point. For now I'm OK with not including them.


class Undefined: ...

def default_environment() -> Dict[str, str]: ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also format_full_version


class Requirement:
name: str = ...
url: str = ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional[str]

class Requirement:
name: str = ...
url: str = ...
extras: str = ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a set, probably of strings but I haven't checked.

name: str = ...
url: str = ...
extras: str = ...
specifier: str = ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a packaging.specifiers.SpecifierSet


class SpecifierSet(BaseSpecifier):
def __init__(self, specifiers: str = ..., prereleases: Optional[bool] = ...) -> None: ...
def __hash__(self) -> int: ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The concrete __str__ method will have to be added here and to _IndividualSpecifier, or mypy will disallow instantiation of this class.

@@ -0,0 +1,36 @@
from typing import FrozenSet, Iterable, Iterator, Optional, Sequence, Tuple

PythonVersion = Sequence[int]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in the previous file, these should be private.

python_version: Optional[PythonVersion] = ...,
abis: Optional[Iterable[str]] = ...,
platforms: Optional[Iterable[str]] = ...,
**kwargs: bool,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the other **kwargs function actually take a keyword-only parameter warn: bool = ...; the **kwargs are a convoluted mechanism to support keyword-only arguments in Python 2. Keyword-only syntax works in stubs regardless of Python version, so we should just write *, warn: bool = ....


from .version import Version

NormalizedName = NewType("NormalizedName", str)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also should be private

@@ -0,0 +1,70 @@
from typing import Optional, Tuple, Union
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERSION_PATTERN: str is in __all__ and should be in the stub.

@brettcannon
Copy link
Member

Yes I'm one of the maintainers of 'packaging'.

The project is fully typed via type comments (to the point that we actually export some type variables to help users type their own code), so if there are issues with the types I would rather address them upstream so everyone gets the direct benefit and we can make sure there the type hints stay accurate. I will also say that those type comments will become type annotations once we drop Python 2 support either by the end of this year or early next year (it will be synced up with when pip drops Python 2).

@erictraut erictraut closed this Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants