Skip to content

Conversation

@ilevkivskyi
Copy link
Member

@gvanrossum Could you please take a look at this? (when you have time, this is not urgent)

Fixes python/typing#367

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I am still not feeling great, so apologies for not catching up on perhaps more urgent PRs.

pep-0484.txt Outdated
common practice in languages with generics (e.g. Java, TypeScript).

Generic versions of concrete collections (except for built-in classes)
could be instantiated::
Copy link
Member

Choose a reason for hiding this comment

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

could->can (or may)

-- ``IntNode`` and ``StrNode`` are distinguishable class objects, but
the type (class) of the objects created by instantiating them doesn't
record the distinction. This behavior is called "type erasure"; it is
common practice in languages with generics (e.g. Java, TypeScript).
Copy link
Member

Choose a reason for hiding this comment

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

The previous paragraphs suggest that you have to create a type alias (e.g. IntNode) but that's no longer necessary (since it's no longer as slow). Maybe you can update that while you're at it? I'd still recommend using a type alias (since it's clearer and still a tiny bit faster) but it's not strictly necessary.

pep-0484.txt Outdated
data = DefaultDict[int, bytes]()

Note that one should not confuse static types and runtime classes.
Type is still erased in this case and the above expression is
Copy link
Member

Choose a reason for hiding this comment

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

Type -> The type

pep-0484.txt Outdated
record the distinction. This behavior is called "type erasure"; it is
common practice in languages with generics (e.g. Java, TypeScript).

Generic versions of concrete collections (except for built-in classes)
Copy link
Member

Choose a reason for hiding this comment

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

"built-in classes" is a little ambiguous. I think it refers to exactly List, Set and Dict, right? If so, I would just list those. Also mention that user-defined subclasses thereof can be instantiated.

(Tuple is excluded for a different reason, it's not a regular class.)

@ilevkivskyi
Copy link
Member Author

@gvanrossum Thank you for review! I have rewritten the part on type aliases as you proposed and listed explicitly types that can and cannot be instantiated.

pep-0484.txt Outdated

Note that the runtime type (class) of ``p`` and ``q`` is still just ``Node``
-- ``Node[int]`` and ``Node[str]`` are distinguishable class objects, but
the type (class) of the objects created by instantiating them doesn't
Copy link
Member

Choose a reason for hiding this comment

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

I'd use "runtime class" rather than "type (class)" here.

pep-0484.txt Outdated

Generic versions of abstract collections like ``Mapping`` or ``Sequence``
and generic versions of built-in classes -- ``List``, ``Dict``, ``Set``,
and ``FrozenSet`` cannot be instantiated. However, user-defined subclasses
Copy link
Member

Choose a reason for hiding this comment

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

Need another "--" before "cannot". Also perhaps add "concrete" before "user-defined"?

@ilevkivskyi
Copy link
Member Author

@gvanrossum I fixed the remaining things in a new commit.

@gvanrossum gvanrossum merged commit ccae12b into python:master Feb 1, 2017
@gvanrossum gvanrossum deleted the clarify-type-erasure branch February 1, 2017 00:27
@gvanrossum
Copy link
Member

Thanks! Still need to hold back reviewing your other stuff, sorry.

@ilevkivskyi
Copy link
Member Author

No problem, take your time. (I will probably produce more stuff in the meantime).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants