(🎁) conformance: NewType isn't a class
#2015
Conversation
KotlinIsland
changed the title
Newtype isn't a classNewtype isn't a class
conformance/tests/aliases_newtype.py
Outdated
| @@ -14,6 +14,8 @@ | |||
|
|
|||
| assert_type(UserId(5) + 1, int) | |||
|
|
|||
| UserId.mro() # E: NewType is not an instance of `type` | |||
There was a problem hiding this comment.
By my reading of the spec, this shouldn't result in a type error. Can you point to the text in the spec that would indicate that it should? All conformance tests should be motivated by the language in the typing spec. If you think that there's a hole in the typing spec, please work through the process to change the spec.
There was a problem hiding this comment.
By my reading of the spec, this shouldn't result in a type error.
where in the spec does it say that the return value of NewType is an instance of type?
While at runtime,
NewType('Derived', Base)returns a dummy function that simply returns its argument
The function returned by
NewType
since function objects don’t support these operations.
seems pretty clear to me that it returns a function
There was a problem hiding this comment.
That's a reasonable point. I think this is somewhat of a gray area, but it is implied by the current wording in the typing spec even though it only explicitly mentions "Both isinstance and issubclass, as well as subclassing will fail...".
@JelleZijlstra, what do you think about including this?
@KotlinIsland, in general, we want to make sure that the conformance tests are well motivated by the text of the spec, but there are admittedly some gray areas where the spec implies something without stating it. You'll see that most tests actually include a quote from the spec so it's clear what part they're testing.
There was a problem hiding this comment.
at runtime it is a function, as the spec says it must be. so i don't see how this is grey at all
i think it's clear that the spec is covering isinstance and subclassing to rule out the idea that is could be a type, not to subtly imply that it is a type
There was a problem hiding this comment.
i think it would benefit the spec for type statments to include similar wording, such to specify that type statements will return a TypeAliasType, not a type, and they they are not suported by isinstance and subtyping
There was a problem hiding this comment.
I'd prefer for the spec to not go into too much detail about how things are implemented at runtime; that can change between Python versions and keeping the details up to date in the spec doesn't seem useful.
I wouldn't be opposed to saying that a NewType isn't an instance of type, though. We could also add a test enforcing the same thing. I'd prefer to test it with something like _: type = UserId instead of relying on the .mro method.
There was a problem hiding this comment.
great, this has been updated and pushed
KotlinIsland
changed the title
Newtype isn't a classNewType isn't a class
i can't update the results on my machine, sorry!