Skip to content

PEP xxx: Inner Fields Annotations #3326

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

Closed
wants to merge 2 commits into from

Conversation

UltimateLobster
Copy link

@UltimateLobster UltimateLobster commented Sep 1, 2023

Appologies for the long time, as discussed in https://discuss.python.org/t/26527, I created a PEP for this suggestion.

I tried to follow the guidelines but this is my first time writing a PEP, so I'm sorry if some things do not quite follow the conventions.

Basic requirements (all PEP Types)

  • Read and followed PEP 1 & PEP 12
  • File created from the latest PEP template
  • PEP has next available number, & set in filename (pep-NNNN.rst), PR title (PEP 123: <Title of PEP>) and PEP header
  • Title clearly, accurately and concisely describes the content in 79 characters or less
  • Core dev/PEP editor listed as Author or Sponsor, and formally confirmed their approval
  • Author, Status (Draft), Type and Created headers filled out correctly
  • PEP-Delegate, Topic, Requires and Replaces headers completed if appropriate
  • Required sections included
    • Abstract (first section)
    • Copyright (last section; exact wording from template required)
  • Code is well-formatted (PEP 7/PEP 8) and is in code blocks, with the right lexer names if non-Python
  • PEP builds with no warnings, pre-commit checks pass and content displays as intended in the rendered HTML
  • Authors/sponsor added to .github/CODEOWNERS for the PEP

Standards Track requirements

  • PEP topic discussed in a suitable venue with general agreement that a PEP is appropriate
  • Suggested sections included (unless not applicable)
    • Motivation
    • Rationale
    • Specification
    • Backwards Compatibility
    • Security Implications
    • How to Teach This
    • Reference Implementation
    • Rejected Ideas
    • Open Issues
  • Python-Version set to valid (pre-beta) future Python version, if relevant
  • Any project stated in the PEP as supporting/endorsing/benefiting from the PEP formally confirmed such
  • Right before or after initial merging, PEP discussion thread created and linked to in Discussions-To and Post-History

📚 Documentation preview 📚: https://pep-previews--3326.org.readthedocs.build/pep-0728/

@UltimateLobster UltimateLobster requested a review from a team as a code owner September 1, 2023 11:39
@AA-Turner AA-Turner changed the title PEP-728 - Initial draft PEP 728: Inner Fields Annotations Sep 1, 2023
@@ -0,0 +1,322 @@
PEP: 728
Title: Inner Fields Annotations
Copy link
Member

Choose a reason for hiding this comment

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

Could you be a bit more specific with this title? Before reading the PEP I had no idea what "Inner Fields Annotations" meant.

PEP: 728
Title: Inner Fields Annotations
Author: Nir Schulman <[email protected]>
Sponsor: Jelle Zijlstra <[email protected]>
Copy link
Member

Choose a reason for hiding this comment

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

ping @JelleZijlstra for confirmation of sponsorship

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, but I am not sponsoring this PEP at the moment. I already have four open PEPs (696, 702, 724, 727), that feels like too much. I can think about sponsoring more when a few of the others are done.

Copy link
Author

Choose a reason for hiding this comment

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

I see. In that case I'll wait and in the meantime address the rest of the comments (and in general try to improve the idea).

@AA-Turner Does this mean this PR should be closed for now or should it stay open with an appropriate label?

Copy link
Member

Choose a reason for hiding this comment

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

@UltimateLobster I would suggest that you revisit the Discourse thread and see if another core developer interested in typing would sponsor this proposal. I'll close this PR for now so that we're aware that the PEP number isn't reserved, though still happy to discuss any of the comments I made in review.

A

Copy link
Member

Choose a reason for hiding this comment

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

Just to set expectations, I don't know whether I'd be willing to sponsor the PEP if I had fewer open PEPs; I'd have to think more about whether I think this is a good direction to go into. I'd also prefer to see more evidence of the feasibility of the idea (e.g., buy-in from type checker maintainers, ideally a prototype implementation).

Copy link
Member

Choose a reason for hiding this comment

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

Please wrap to 80 columns per PEP 1

==========
Consider the following example:

.. code-block:: py
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
.. code-block:: py
.. code-block:: python


The latter problem can be solved using ``typing.overload``:

.. code-block:: py
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
.. code-block:: py
.. code-block:: python

the type of ``obj['x']``. These types would have the same meaning for all python objects (Whether working with TypedDicts or
dataclasses). It would also allow referring to fields of objects containing both attributes and items:

.. code-block:: py
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
.. code-block:: py
.. code-block:: python

Comment on lines +301 to +302
Should __annotations__ be used as the reference table for fields?
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
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
Should __annotations__ be used as the reference table for fields?
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
Should __annotations__ be used as the reference table for fields?
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''

Comment on lines +313 to +314
Naming
''''''
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
Naming
''''''
Naming
''''''

Copyright
=========

This document is placed in the public domain or under the CC0-1.0-Universal license, whichever is more permissive.
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
This document is placed in the public domain or under the CC0-1.0-Universal license, whichever is more permissive.
This document is placed in the public domain or under the
CC0-1.0-Universal license, whichever is more permissive.

func("something else")



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

@AA-Turner AA-Turner added the new-pep A new draft PEP submitted for initial review label Sep 1, 2023
Title: Inner Fields Annotations
Author: Nir Schulman <[email protected]>
Sponsor: Jelle Zijlstra <[email protected]>
Discussions-To: https://discuss.python.org/t/26527
Copy link
Member

Choose a reason for hiding this comment

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

This link should go into Post-History; a new thread should be created for PEP 728 just before this PR is merged

========

This PEP specifies a way for annotations to reference the field types of other complex objects. This include the field
names and types objects with static layout such as dataclasses (or dataclass-like objects), TypedDicts and NamedTuples.
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
names and types objects with static layout such as dataclasses (or dataclass-like objects), TypedDicts and NamedTuples.
names and types objects with static layout such as dataclasses (or dataclass-like objects), :py:class:`~typing.TypedDict`s and :py:class:`~typing.NamedTuple`s.

@AA-Turner
Copy link
Member

@UltimateLobster I've added the new PEP checklist to the PR body, please may you go through it and fill in as appropriate?

A

bar: int
baz: str

bar: TypeAlias = Literal['baz']
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this use the type parameter syntax since TypeAlias is deprecated(-ish)? They technically don't do the same thing at runtime but I'm guessing both are supposed to work the same within the context of this PEP's new typing constructs.

@AA-Turner
Copy link
Member

Closing per discussion (no sponsor)

A

@AA-Turner AA-Turner closed this Sep 2, 2023
@Kludex
Copy link

Kludex commented Sep 6, 2023

Just to set expectations, I don't know whether I'd be willing to sponsor the PEP if I had fewer open PEPs; I'd have to think more about whether I think this is a good direction to go into. I'd also prefer to see more evidence of the feasibility of the idea (e.g., buy-in from type checker maintainers, ideally a prototype implementation).

I know you didn't mean for other packages, but I would use this in Starlette (FastAPI dependency) as soon as it becomes available.

Use case is described on python/typing#1457.

FYI: I'm not trying to reopen this, I'm just randomly commenting.

@UltimateLobster
Copy link
Author

Just to set expectations, I don't know whether I'd be willing to sponsor the PEP if I had fewer open PEPs; I'd have to think more about whether I think this is a good direction to go into. I'd also prefer to see more evidence of the feasibility of the idea (e.g., buy-in from type checker maintainers, ideally a prototype implementation).

I know you didn't mean for other packages, but I would use this in Starlette (FastAPI dependency) as soon as it becomes available.

Use case is described on python/typing#1457.

FYI: I'm not trying to reopen this, I'm just randomly commenting.

Thank you for the reply!
I am working on a prototype PR right now as well as checking the opinion of different type checker maintainers

@hugovk hugovk changed the title PEP 728: Inner Fields Annotations PEP xxx: Inner Fields Annotations Sep 25, 2023
@Badg
Copy link

Badg commented Apr 23, 2024

FWIW, this would allow correct typing of dataclasses.replace (and similar operations, which can be crucial when defining APIs that operate over frozen dataclasses) without any special-casing in type checking, see here

@hugovk
Copy link
Member

hugovk commented Apr 23, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-pep A new draft PEP submitted for initial review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants