Skip to content

PEP 484: Allow annotating first argument of instance and class methods #89

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

Merged
merged 3 commits into from
Nov 1, 2016

Conversation

ilevkivskyi
Copy link
Member

@ilevkivskyi ilevkivskyi commented Sep 3, 2016

Fixes python/typing#271 and python/mypy#1212

@JukkaL @gvanrossum
Please review.

@JukkaL The third example works in mypy (with appropriate function bodies), Guido wants to double check that mypy could be patched so that first two examples also work.

@@ -109,7 +109,7 @@ It is recommended but not required that checked functions have
annotations for all arguments and the return type. For a checked
function, the default annotation for arguments and for the return type
is ``Any``. An exception is that the first argument of instance and
Copy link
Member

Choose a reason for hiding this comment

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

Drop "that".

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@ilevkivskyi
Copy link
Member Author

@gvanrossum I implemented your comments in the new commit.


T = TypeVar('T', bound='C')
class C:
def factory(cls: Type[T]) -> T:
Copy link
Member

Choose a reason for hiding this comment

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

Missing @classmethod?

Copy link
Member Author

Choose a reason for hiding this comment

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

@markshannon
Oops!
Thank you, fixed this.

@JukkaL
Copy link
Contributor

JukkaL commented Sep 5, 2016

This looks good to me, and I believe that it won't be very hard to get this working in mypy. But I haven't actually done more than thought experiments.

@gvanrossum
Copy link
Member

Let's wait until we've prototyped this successfully in mypy.

@gvanrossum
Copy link
Member

In python/mypy#2193 (comment), @elazarg expresses some doubts about the feasibility of this approach. I haven't thoroughly ingested his comments yet but I thought I'd call it out here.

@elazarg
Copy link

elazarg commented Oct 4, 2016

I didn't mean to question the feasibility of anything. I only say we need to distinguish between

  • Mixins that magically copy / create instances of their dynamic types, and
  • Interfaces that require overriding methods to return an instance of their static type.

These are two different things, and need different syntax.

@elazarg
Copy link

elazarg commented Oct 4, 2016

I also think the syntax suggested here is more appropriate for the interface thing, while the examples in this PEP change seems to be about mixins. It is of course somewhat a matter of taste, but I think it is not entirely subjective.

@ilevkivskyi
Copy link
Member Author

@gvanrossum @elazarg To be honest, I still could not clearly see the difference between the two use cases (probably it is to much blurred for me). But what I could say is in my opinion both examples in the PR (and two examples Guido gave in mypy tracker) if what we are interested in. Also I think the syntax should be self: T, at least for me it is clear what it means (it more-or-less simply allows generic function semantics for methods).

@gvanrossum
Copy link
Member

gvanrossum commented Oct 5, 2016 via email

@ilevkivskyi ilevkivskyi changed the title [WIP] PEP 484: Allow annotating first argument of instance and class methods PEP 484: Allow annotating first argument of instance and class methods Oct 29, 2016
@ilevkivskyi
Copy link
Member Author

@gvanrossum Since support for this feature (at least the functionality described in this PR) is now in mypy python/mypy#2193 , I think this PR could be merged.

@gvanrossum gvanrossum merged commit ada7d35 into python:master Nov 1, 2016
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.

6 participants