typing icon indicating copy to clipboard operation
typing copied to clipboard

Modified `generics_type_erasure.py` test to not require an error when…

Open erictraut opened this issue 8 months ago • 6 comments

… accessing a generic attribute on a class when a subtype could provide specialization for that generic attribute.

Pyright recently made a modification to eliminate a false positive error in this case. See https://github.com/microsoft/pyright/discussions/10303 for details. I don't think the conformance test should mandate an error in this case. The spec certainly doesn't say anything about it, and the error is potentially a false positive.

erictraut avatar May 20 '25 01:05 erictraut

@JelleZijlstra, I think you wrote this test originally. Could you review the proposed change and let me know what you think?

erictraut avatar May 20 '25 01:05 erictraut

Looks like @hauntsaninja wrote this in #1589.

The spec itself has a similar line in https://typing.python.org/en/latest/spec/generics.html#instantiating-generic-classes-and-type-erasure (look for type(p).x). I'm sympathetic to allowing this but we should change the spec example too.

JelleZijlstra avatar May 20 '25 02:05 JelleZijlstra

And in fact the example comes from PEP 484: https://peps.python.org/pep-0484/#instantiating-generic-classes-and-type-erasure

JelleZijlstra avatar May 20 '25 02:05 JelleZijlstra

Yup, I added the test because of the relevant line in PEP 484 / the spec.

This PR makes sense given that none of pytype/pyre/mypy error here. I am sympathetic to the use case in the pyright discussion, since I can't think of a great alternative for the user. If we want to try and close down the hole introduced, we could consider erroring on assignment to such variables.

hauntsaninja avatar May 20 '25 02:05 hauntsaninja

Ah good catch. I didn't see that in the spec. Yeah, we should probably fix this in the spec then. One way to do this is to simply delete that line in the example. I'm not sure we want to mandate or disallow an error in this case, so leaving it up to type checkers is probably the right answer.

@JelleZijlstra, do you think that deleting this line is a substantive enough change that we should go through the full process of soliciting feedback and having the full TC sign off on it? Or is it small enough that we can shortcut that process? I doubt it will be contentious.

erictraut avatar May 20 '25 03:05 erictraut

I feel given that we're changing the output of something that has been explicitly specified to raise an error, this is a substantive change and should go through Council approval.

JelleZijlstra avatar May 20 '25 14:05 JelleZijlstra