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

PEP 696: Type defaults for TypeVars #2717

Merged
merged 13 commits into from
Jul 24, 2022

Conversation

Gobot1234
Copy link
Contributor

Proposes Type defaults for TypeVars, a way to specify the default type for an omitted type parameter:

from dataclasses import dataclass
from typing import TypeVar

 T = TypeVar("T", default=int)  # This means that if no type is specified T = int

 @dataclass
 class Box(Generic[T]):
     value: T | None = None

 reveal_type(Box())                      # type is Box[int]
 reveal_type(Box(value="Hello World!"))  # type is Box[str]

I've had a thread on this on typing-sig for quite some time I just didn't have enough time to finalise all of the semantics, https://mail.python.org/archives/list/[email protected]/thread/7VWBZWXTCX6RAJO6GG67BAXUPFZ24NTC.

CC @JelleZijlstra

@Gobot1234 Gobot1234 requested a review from a team as a code owner July 16, 2022 01:13
.github/CODEOWNERS Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated
Comment on lines 83 to 84
where the parameter should ``start`` default to ``int``, ``stop``
default to ``start`` and step default to ``int | None``
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
where the parameter should ``start`` default to ``int``, ``stop``
default to ``start`` and step default to ``int | None``
where the parameter ``start`` should default to ``int``, ``stop`` shouldd
default to the type of ``start`` and ``step`` should default to ``int | None``

Also, do you mean stop should also default to int?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No I thought for convince sake they'd be the same type most of the time so it'd save people the extra type 99% of the time

pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated
.. code:: py

StartT = TypeVar("StartT", default=int)
StopT = TypeVar("StopT", default=StartT)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we should allow a TypeVar as the default for another TypeVar; this may make the TypeVar difficult to solve for in some cases. @erictraut do you have any opinion here?

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 something that typescript supports so I naively thought it can't be an impossible feature to support

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this makes sense. A TypeVar used as a type argument must be associated with a scope (a generic class, function, or type alias). What scope is this type variable associated with? Or are you saying that this would be possible only in cases where the TypeVar is already associated with an outer scope?

class Outer[T]:
    class Inner[X = T]:
        ...

I think the use cases for this are very rare, and it adds significant complexity — in the type checker implementation and in the specification (more edge cases to reason about), so I'd recommend against including this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it would need to have the TypeVar it's defaulting to bound to an outer scope so you could also do

class slice[StartT = int, StopT = StartT, StepT = int | None]: ...

slice[str]()  # type is slice[str, str, int | None]

(in TS)

class Slice<StartT = number, StopT = StartT, StepT = number | null> {};

new Slice<string>()  // type is Slice<string, string, number | null>

I don't think there are that many edge cases to reason with here, if a TypeVar with a default (T2) that is another TypeVar (T1) then T1 needs to be in scope before T2 can be used otherwise it's an error.

i.e.

class slice[StartT = StopT, StopT = int, StepT = int | None]: ...

doesn't work, I think this fits in quite neatly with PEP 695's handling of this.

Copy link
Contributor

@erictraut erictraut Jul 18, 2022

Choose a reason for hiding this comment

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

No, this won't work for PEP 695. Because of all the runtime constraints that are imposed on the implementation of PEP 695, I need to allocate all of the type parameters before binding them to their names. That means the TypeVars StartT, StopT, and StepT in your example above need to be constructed prior to referring to any of them by name. So the default expressions won't be able to refer to other type parameters.

Unless we have some really strong use cases in mind for this, I'd prefer to remove it from the PEP.

Copy link
Contributor

@erictraut erictraut Jul 18, 2022

Choose a reason for hiding this comment

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

I guess we could support this in the PEP 695 implementation if we construct the type variables first, then evaluate the default expressions and "inject" the result into the already-constructed type variable. The equivalent of this...

__local_typevars__ = (TypeVar("StartT"), TypeVar("StopT"), TypeVar("StepT"))
__local_typevars__[0].__default__ = int
__local_typevars__[1].__default__ = __local_typevars__[0]
__local_typevars__[2].__default = int | None

Gobot1234 and others added 2 commits July 16, 2022 14:24
@Gobot1234 Gobot1234 changed the title PEP 9999: New PEP "Type defaults for TypeVars" PEP 696: Type defaults for TypeVars Jul 16, 2022
pep-0696.rst Show resolved Hide resolved
pep-0696.rst Outdated Show resolved Hide resolved
pep-0696.rst Outdated Show resolved Hide resolved
pep-0696.rst Outdated Show resolved Hide resolved
pep-0696.rst Show resolved Hide resolved
pep-0696.rst Outdated Show resolved Hide resolved
Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

A few little formatting nits.

pep-0696.rst Outdated Show resolved Hide resolved
pep-0696.rst Outdated Show resolved Hide resolved
pep-0696.rst Outdated Show resolved Hide resolved
pep-0696.rst Outdated Show resolved Hide resolved
pep-0696.rst Outdated Show resolved Hide resolved
pep-0696.rst Outdated Show resolved Hide resolved
pep-0696.rst Outdated Show resolved Hide resolved
Co-authored-by: Hugo van Kemenade <[email protected]>
@Gobot1234
Copy link
Contributor Author

OK, I now understand what you meant by this statement. I didn't understand previously, which is why I asked for an example. Now that I understand, I don't think rule should exist. There shouldn't be any interaction between TypeVar defaults and argument defaults. A TypeVar default applies only in cases where the type of the type argument cannot otherwise be solved or is not specified. If a default argument value is provided, then there is a way for the constraint solver to solve the type for DefaultIntT, so its default type shouldn't apply.

So my recommendation is that this entire section should be deleted.

To clarify do you mean defaults shouldn't be useable in functions at all or just to delete the compatibilty detection part?

@erictraut
Copy link
Contributor

I'm recommending that the entire section "Function Defaults" should be deleted. This section currently says "The TypeVar's default should also be compatible with the parameter's runtime default if present...". I don't see any justification for this limitation. Unless I'm missing something, the default argument for a function parameter is completely unrelated to the default type of a TypeVar that happens to appear somewhere in the parameter's type annotation. Therefore, there should be no limitation that relates the two.

A TypeVar default type is applicable in only two cases:

Case 1. During explicit specialization where a type argument is omitted

class Foo[S, T = int]: ...
reveal_type(Foo[str]()) # Revealed type is "Foo[str, int]"

# If we allow explicit specialization of generic functions in the future...
def foo[S, T = int](a: S, b: T) -> S | T: ...
reveal_type(foo[str]) # Revealed type is "(a: str, b: int) -> str | int"

Case 2. When solving TypeVars in a call expression and the TypeVar remains unsolved

class Foo[T = str]:
    def __init__(self, val: Iterable[T] | None = None) -> None: ...
reveal_type(Foo())  # Foo[str]

def foo[T = str](a: T | None = None) -> T: ...
reveal_type(foo(1)) # Revealed type is int
reveal_type(foo()) # Revealed type is str

Neither case 1 or 2 apply in the examples you've provided in the "Function Defaults" section, so the TypeVar default type doesn't apply in these cases. It should therefore be ignored, not flagged as an error.

@srittau
Copy link
Contributor

srittau commented Jul 19, 2022

I see that there's already a lot of discussion about this PEP in the PR. Could we get a first version merged so that we can all discuss the PEP together in a proper forum?

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Otherwise let's get this draft merged for discussion!

pep-0696.rst Outdated Show resolved Hide resolved
Co-authored-by: Hugo van Kemenade <[email protected]>
pep-0696.rst Outdated Show resolved Hide resolved
pep-0696.rst Outdated Show resolved Hide resolved
pep-0696.rst Show resolved Hide resolved
pep-0696.rst Outdated Show resolved Hide resolved
pep-0696.rst Outdated Show resolved Hide resolved
@JelleZijlstra
Copy link
Member

I see that there's already a lot of discussion about this PEP in the PR. Could we get a first version merged so that we can all discuss the PEP together in a proper forum?

As the sponsor, I'd like to make sure the PEP is something I can stand behind before merging it. There are still a few small points I can't support.

pep-0696.rst Outdated Show resolved Hide resolved
@JelleZijlstra JelleZijlstra merged commit 49686bc into python:main Jul 24, 2022
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.

5 participants