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

Allow bare typing.ClassVar qualifiers #1931

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Viicos
Copy link
Contributor

@Viicos Viicos commented Feb 24, 2025

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.

The substance of this looks good to me, thanks for the PR!

Comment on lines 34 to 35
inferred as some type to which this value is :term:`assignable` (in this
case, either ``int``, ``Literal[2]``, or ``Any``).
Copy link
Member

Choose a reason for hiding this comment

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

The listed options are not the only choices here, I'd prefer not to imply that they are. (I think a strong case can be made that inferring Any | Literal[2] is the most correct option, from a gradual-typing perspective.) I'm fine with just listing the simpler options that you have here, but we shouldn't imply that it's an exhaustive list.

Suggested change
inferred as some type to which this value is :term:`assignable` (in this
case, either ``int``, ``Literal[2]``, or ``Any``).
inferred as some type to which this value is :term:`assignable` (for example,
``int``, ``Literal[2]``, or ``Any``).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what I wanted to write out to begin with, but I realize my wording is wrong indeed :)

case, either ``int``, ``Literal[2]``, or ``Any``).

If the ``ClassVar`` qualifier is used without any assigned value, the type
should be inferred to an unknown static type (such as :ref:`Any`).
Copy link
Member

Choose a reason for hiding this comment

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

The only dynamic type discussed in the typing spec is Any, so it's a bit odd to imply here that there would be others. Most type checkers do distinguish between different "origins" of Any, and some do so with a variant of Any named Unknown, but as far as the spec is concerned these are all just Any.

Suggested change
should be inferred to an unknown static type (such as :ref:`Any`).
should be inferred as :ref:`Any`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was wondering if such a concept of an unknown static type, as written in the referenced any section:

.. _`any`:
``Any``
-------
``Any`` represents an unknown static type.
Every type is :term:`assignable` to ``Any``, and ``Any`` is assignable to every
type.

would be worth specifying, and that's why I chose the same phrasing. But let's not complicate things, I'll go with Any.

@erictraut
Copy link
Collaborator

This change to the spec looks reasonable to me. If this is accepted, someone will need to write conformance tests for it. @Viicos, if you're willing, you could include the conformance test updates in the same PR.

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.

3 participants