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

Merge the specification portion of the "Type Stubs" doc into the spec #1815

Merged
merged 46 commits into from
Aug 21, 2024

Conversation

rchen152
Copy link
Collaborator

@rchen152 rchen152 commented Jul 22, 2024

The Type Stubs doc describes allowed elements for stub files. As a standalone doc, it doesn't really fit in with the rest of the typing documentation. This PR merges its content into the spec:

  • The "Introduction," "Syntax," and "Supported Constructs" sections have been moved into the typing specification.
  • "Distribution" has been deleted, as the entirety of this section was basically "See [link]."
  • Some content that was repeated across this doc, the spec, and the "Typing Python Libraries" guide has been deduplicated.

For #1605.

* Moves some content into spec, deletes duplicate spec content.
* Adds placeholders for where remaining content will be moved. It will be
  split between the spec and the writing_stubs doc.
Moves the section on what to include in stubs with editorial changes only -
e.g., changing "type stub" to "stub file" to match surrounding terminology.
Moves the style guide with a few updates:
* Editorial changes like tweaking code examples to use recommended style.
* Attribute declarations can use assignments when needed to convey extra
  information - e.g., final attributes and enum members.
* Tweaks a reference to the double-underscore convention for positional-only
  arguments to note that it's historical.
The only substantive change is an update to "Comments" to note that many
formats of error suppression comments are allowed.
Added `x: Final = <literal>` to module-level attributes section.
No other changes.
Replaces outdated info with a link to the enums section of the spec.
Added an example of an inner class to "Classes." Removed mention of
asyncio.coroutine from "Decorators" as it's very deprecated. Added functions
decorated with `@dataclass_transform` to list of decorators that type
checkers should recognize.
Removed mention of individual supported/unsupported features from this
section. Where we state that all typing features from the latest released
version are supported, added a caveat with a link to the typeshed feature
tracker.
Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

Thanks for splitting this doc! A few notes below.

I also think that the spec part of this needs more love in the future (not in this PR) to remove redundant information or replace it with links to other sections. Some of the text also probably better belongs in the guide, or should at least be duplicated there.

We should also not completely remove the old stubs document, because that leads to 404 errors. Instead I recommend a short document with a text similar to this: "The contents of this document have been moved here: [...]". I think by adding the :orphan: directive we can prevent this document to be linked in the TOC and suppress the corresponding, but I haven't tried this myself.

I've added a few comments marked "Future note". I don't think they should be included in this PR (to keep it focused), but we should address these points in a followup PR. (I can work on that.)

docs/guides/libraries.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@erictraut erictraut left a comment

Choose a reason for hiding this comment

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

I focused my review on the typing spec section, so my comments are limited to that portion of the PR.

I think it's important to break this effort into two separate PRs — one for the typing spec (which the typing council will need to approve) and one for the guides documentation (which requires less formality and scrutiny). These two sections are each very significant, they target different audiences, and they have different review/approval processes.

For the typing spec, I think we need to be careful to include only information that pertains to type checker behaviors. The current PR contains a bunch of guidance directed at stub authors. This should be moved to guides, not included in the typing spec.

The current PR also contains a bunch of information that is covered already in other parts of the typing spec. In many cases, this new section describes existing information in incomplete and/or contradictory terms. I think all of the redundant information needs to be stripped out. It's fine to include references to other parts of the typing spec, but it the new section should not attempt to repeat the information from other sections. This will cause confusion and lead to inconsistencies in the spec.

The source material for this PR was written before "py.typed" libraries became popular. There are many similarities between stub files and inlined type information found within a "py.typed" library. I think the typing spec needs to describe the behaviors of type checkers with regard to both stubs and "py.typed" libraries. Given the large overlap between the two, these should be covered in the same section, and we should strive to reduce any differences between the two cases. This document (which is a peer to the "Writing Stubs" document used as the source material for this PR) is my attempt to document the intended behaviors for "py.typed" libraries. I think the information in both of these documents needs to be consolidated into the typing spec.

docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Show resolved Hide resolved
* ``property`` (including ``.setter``)
* ``abc.abstractmethod``
* ``dataclasses.dataclass``
* functions decorated with ``@typing.dataclass_transform``
Copy link
Collaborator

Choose a reason for hiding this comment

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

Plus classes decorated with @typing.dataclass_transform.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought that classes decorated with dataclass_transform were inherited from, not used as decorators?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's correct, but the dataclass_transform is used then as a decorator on the metaclass or base class. This class decorator must be supported by type checkers. The current wording above says "functions decorated with @typing.dataclass_transform".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's covered by "all decorators defined in the typing module" (in the paragraph above this list).

Copy link
Member

Choose a reason for hiding this comment

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

I also had this question while reading this section. Probably

* functions decorated with ``@typing.dataclass_transform``

can be deleted, since

Type checkers are expected to understand the effects of all decorators defined
in the ``typing`` module, plus these additional ones:

covers it.

Copy link
Member

@carljm carljm Aug 21, 2024

Choose a reason for hiding this comment

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

I think the reasoning behind the inclusion of this bullet, with its current wording, is that a function decorated with typing.dataclass_transform is itself a decorator (which is not itself defined in the typing module), whose usage must be understood by type checkers. So technically this case is not covered simply by understanding all decorators defined in the typing module.

A class decorated by typing.dataclass_transform is not itself a decorator, so the class case is fully covered by "all decorators defined in the typing module."

docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
@srittau
Copy link
Collaborator

srittau commented Jul 22, 2024

I think it's important to break this effort into two separate PRs — one for the typing spec (which the typing council will need to approve) and one for the guides documentation (which requires less formality and scrutiny). These two sections are each very significant, they target different audiences, and they have different review/approval processes.

I agree. When we start by splitting out the non-spec part first, we can also more easily merge non-spec behavior from the spec part into the guides section.

@rchen152
Copy link
Collaborator Author

Thanks for the quick and thorough reviews! I'll address these comments, then break out the non-spec changes into another PR and aim to get those merged first.

@rchen152 rchen152 marked this pull request as draft July 23, 2024 06:35
Co-authored-by: Sebastian Rittau <srittau@rittau.biz>
Copy link

cpython-cla-bot bot commented Jul 23, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

rchen152 and others added 9 commits July 22, 2024 23:37
Co-authored-by: Sebastian Rittau <srittau@rittau.biz>
Co-authored-by: Sebastian Rittau <srittau@rittau.biz>
Co-authored-by: Sebastian Rittau <srittau@rittau.biz>
Co-authored-by: Sebastian Rittau <srittau@rittau.biz>
Co-authored-by: Sebastian Rittau <srittau@rittau.biz>
rchen152 added a commit to rchen152/typing that referenced this pull request Jul 24, 2024
This is part of an effort to integrate the standalone "Type Stubs" doc
into the rest of the typing documentation. See
python#1815 for more context.

For python#1605.
@rchen152 rchen152 requested a review from erictraut August 9, 2024 00:59
Copy link
Collaborator

@erictraut erictraut left a comment

Choose a reason for hiding this comment

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

This is looking good. Thanks again for doing this @rchen152. I left a few comments.

docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Outdated Show resolved Hide resolved
implementation, and from the Python version the type checker runs under (if
any).

Type checkers should treat stubs as if ``from __future__ import annotations`` is
Copy link
Member

Choose a reason for hiding this comment

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

This may be useful as a mnemonic but it's not a particularly precise description. For example, forward references in type aliases are valid in stubs, but they would fail at runtime even with the future import enabled. Moreover, from __future__ import annotations is about to be deprecated.

I would suggest something like:

Type checkers should support all type system features in stub files, including those introduced in newer versions. For example, the pipe union syntax (X | Y) introduced in Python 3.10 may be used even before Python 3.9 reaches end-of-life. Type checkers should also support forward references in all contexts, without the need to quote strings. For example, forward references may be used in annotations and in the right-hand side of assignments used as type aliases.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, I like the previous phrasing better. The proposed change doesn't fully capture the intended behavior. It's not just that type checkers should support forward references. They should assume forward references within all type expressions in a stub file, as they do when from __future__ import annotations is used in a source file. The distinction is important because symbol resolution rules change when assuming forward references.

Perhaps we should say that "all type expressions in a stub file should be evaluated as though they are quoted even if they are not".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let me think a bit about how to rephrase this. @JelleZijlstra I see what you mean about the current wording being incorrect, but supporting "all type system features" doesn't seem quite right either. For example, type checkers aren't expected to support the new type keyword (PEP 695) in stubs for a while yet, right, since it's new syntax in 3.12?

Copy link
Collaborator

Choose a reason for hiding this comment

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

A conformant type checker should definitely support the type keyword regardless of whether it's used in a ".pyi" or a ".py" file.

As a stub author, you can decide whether or not to use this feature based on whether the most popular type checkers support this feature, but this is not a concern of the typing spec.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@erictraut What version of Python syntax do you think type checkers ought to support in stubs? Right now, this PR says that stubs use the syntax of the earliest Python version not yet end-of-life (currently 3.8). That's what was suggested here, and I went with it because it seemed sensible and unambiguous. But if we stick with this, it would be contradictory to require type checkers to support newer syntax unless it's a case like the pipe union syntax, where from __future__ import annotations can be used to cheat, basically.

Copy link
Collaborator

Choose a reason for hiding this comment

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

a conformant type checker does not need to accept type T = int in a stub if it runs in Python 3.9 mode.

I don't think that's documented anywhere in the typing spec currently. I don't remember seeing that, and I can't find any such statement in the spec.

This strikes me as an implementation limitation of mypy (because it chooses to use the parser built in to CPython). I think there are multiple compelling reasons why this implementation decision should be revisited by mypy maintainers, so I'd prefer not to codify this in the typing spec.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The fact of the matter is that mypy – despite its shortcomings – is probably the most (explicitly) used and arguably most important type checker.1 In many cases, it's not super important (although of course not desirable) when the current behavior of mypy (or any other type checker for that matter) diverges from the spec, since users will usually use only one type checker for a particular project and can work around the specific idiosyncrasies. But in the case of library interfaces (which type stubs are), conformance is key. This is why I don't think the spec can just ignore mypy in this case – at least not as long there is no change in sight.2

That said, I suggest that the type statement and potentially other newer syntax constructs should be allowed in type stubs, but support should be made optional. (A "MAY" in RFC 2119 parlance.) The guide can then recommend against using those constructs for the time being. The spec can periodically be updated to require support for certain syntax features as mypy (and other type checkers) catch up to support them unconditionally.

Footnotes

  1. pyright is such a fantastic type checker and I wish mypy would come close, especially when it comes to support for new features and response time for bug fixes.

  2. I also wish mypy would switch to an external ast parser and not rely on the ast module. Or at least significantly increase the minimum required Python version to run mypy.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I took a stab at rewording this paragraph; PTAL.

I think it's important for the spec to explicitly state which version of Python syntax should be supported in stubs. Unlike for Python source files, the version isn't necessarily coupled to the runtime, so which one to use is potentially unclear. (Maybe this is already obvious to everyone, but it hadn't been stated yet, so I just wanted to make sure we're on the same page here.)

I agree with Sebastian that this version should be the earliest non-EOL'd version until the situation with mypy changes. One of the purposes of the spec is to "provide library authors guarantees to rely on when working with multiple type checkers." The more the spec diverges from reality, the less useful it becomes for that purpose.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I recognize and appreciate that mypy is important. For that reason, I think it's good for us to provide guidance to stub authors in the "guides" section that they should avoid the use of newer language features if they want to maximize compatibility.

I don't think it makes sense for the typing spec to say that conformant type checkers SHOULD support type statements in ".py" files, but MAY support type statements in stub files. Stub files shouldn't be special in this regard. Conformant type checkers should support all of the features in the latest published version of Python.

I'll note that mypy fails to conform with the typing spec in a number of areas. While that's not ideal, it's understandable. We should make sure that the community understands where mypy is not conformant so they can work around those limitations. Let's make sure we document this, but IMO this documentation doesn't belong in the typing spec.

One option is to amend the typing spec with some variant of what Jelle suggests above: "support for newer typing features that rely on grammar changes is optional if the type checker is implemented in Python and is running on an older version of Python." This statement would apply to both ".py" and ".pyi" files. I personally don't think the spec should say that because it would be codifying a current limitation in mypy. As a TC member, I would vote against such a change, but maybe other TC members would be in favor of it.

I understand why mypy originally leveraged the CPython parser, but that implementation decision has not aged well. And it's an implementation decision that could easily be revisited — especially now that the CPython code base uses the PEG parser. Mypy maintainers could incorporate the latest parser form the CPython code base if this were a priority.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, I think we're at a bit of an impasse here. I understand where you're coming from, but it feels unnecessarily confusing to me for the spec and guides to say different things on supported syntax in stubs.

Discussion on this PR seems to have largely died down, so maybe it's a good time to ask the rest of the typing council to weigh in. I'll make a note of this and other unresolved points when opening the typing council issue.

docs/spec/distributing.rst Outdated Show resolved Hide resolved
docs/spec/distributing.rst Show resolved Hide resolved
docs/spec/distributing.rst Show resolved Hide resolved
rchen152 and others added 2 commits August 12, 2024 17:44
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
rchen152 and others added 2 commits August 19, 2024 20:31
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks! Great work 👏

""""""""""

Type checkers are expected to understand the effects of all decorators defined
in the ``typing`` module, plus these additional ones:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
in the ``typing`` module, plus these additional ones:
in the ``typing`` and ``typing_extensions`` modules, plus these additional ones:

docs/spec/distributing.rst Show resolved Hide resolved
This overrides all other rules above, allowing imported symbols or
symbols whose names begin with an underscore to be included in the
interface.
- Local variables within a function (including nested functions) are
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this point? It would be strange to assume that variables from a function are exported, knowing cpython's behavior.


- ``__all__ = ('a', b')``
- ``__all__ = ['a', b']``
- ``__all__ += ['a', b']``
Copy link
Member

Choose a reason for hiding this comment

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

I would add a note that type-checkers still need to check that += operation has correct types. tuples to tuples, lists to lists, etc.

Suggested change
- ``__all__ += ['a', b']``
- ``__all__ += ['a', b']``

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think that's necessary. Type checkers generally don't type check stubs (i.e. they don't emit errors for stubs that your code imports). Of course, if you ask a type checker to type check a stub, it should perform all of its normal type checks, but that's true of all elements within the stub, not just __all__ += ... statements.

library’s interface. Type checkers can use the following rules
to determine which symbols are visible outside of the package.

- Symbols whose names begin with an underscore (but are not dunder
Copy link
Member

Choose a reason for hiding this comment

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

What about objects @typing.type_check_only? Are they exposed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

typing.type_check_only is orthogonal to whether a symbol should be considered public or private. It means that a type checker should report an error if that symbol is used in a runtime context. These symbols can still be used in type expressions if they are publicly exposed from a library or stub file.

Copy link
Member

@carljm carljm left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for working on this!

@carljm
Copy link
Member

carljm commented Aug 21, 2024

@erictraut You've signed off on this change over at python/typing-council#34, but your review status on this PR is still "changes requested."

@erictraut
Copy link
Collaborator

erictraut commented Aug 21, 2024

@carljm, thanks for noting that. I just marked the PR as approved.

@rchen152, thanks again for all your work on this update! All TC members have signed off on it, so I'll proceed with the merge. You can announce it on the discourse channel if you'd like.

@erictraut erictraut merged commit d2d599f into python:main Aug 21, 2024
5 checks passed
@rchen152 rchen152 deleted the stubs branch August 22, 2024 00:34
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