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

Proposal: inheritance + annotations #269

Closed
dmoisset opened this issue Aug 26, 2016 · 20 comments
Closed

Proposal: inheritance + annotations #269

dmoisset opened this issue Aug 26, 2016 · 20 comments
Labels
resolution: out of scope The idea or discussion was out of scope for this repository topic: feature Discussions about new features for Python's type annotations

Comments

@dmoisset
Copy link
Contributor

After some time annotating non-trivial codebases, I've found something that feels like a nuisance and a bad practice. Essentially, when overriding methods in subclasses, having to re-specify the whole type signatures in subclasses is:

  • type consuming (I'm writing/annotating a subclass and have to copy potentially complicated types for a lot of parameters)
  • error prone
  • not a source new information (which is the benefit of annotations)
  • a violation of the DRY principle (if I choose to change the type of an argument in the superclass, I have to edit all the subclasses).

One cases where this trivially happens is when defining standard python magic methods; I want my __str__ methods checked, but it feels silly to tell the typechecker every time that "yes, this method returns a string". It also happens in more complicated definitions, for example https://github.com/dmoisset/django/blob/typing-requests/django/core/files/uploadhandler.py#L83 and https://github.com/dmoisset/django/blob/typing-requests/django/core/files/uploadhandler.py#L83 vs line 170 of the same file.

Currently, the PEP says «For a checked function, the default annotation for arguments and for the return type is Any». I'd like to introduce some exception to this rule for methods inheriting an annotated method.

I know that there's a potential problem with this that is when inheriting standard classes (especially but not limited to object) which already have annotated clases, may force undesired annotations. My idea here is to introduce some kind of marker (perhaps a @typing.inherit_type_hints decorator?) that can be used at class and method levels and means "inherit annotations from the parent class unless specifically overriden by this class". In this way it's opt-in and also tells both the human reader and the static analyzer where to lookup the annotations.

The implementation of this decorator could be trivial (another identity decorator, just used as a marker for static analysis tools) , but also could actually copy the __anotations__ field to make the inherited information also available to runtime tools.

What do you think of this?

@gvanrossum
Copy link
Member

I mildly like the idea of inheriting the annotations from the superclass instead of defaulting everything to Any, but I don't like the idea of a decorator.

Maybe as an experiment we can modify mypy to behave like this when a special command-line flag is given, and see how noisy it is on partially annotated code?

But I am not bothered as much by the redundancy about which you complain: the annotations provide useful local information for the reader (who would otherwise have to track down the corresponding method in a superclass), and they also actually help when refactoring the code -- if the superclass method changes, a checker can give a clear error reminding you that your signature is incompatible with the superclass method, while in the purely implicit case, you'd get potentially much more mysterious errors about random things in the body of the overriding method.

Lately I seem to get irritated every time DRY gets brought up -- I think as a principle it needs to be examined and perhaps thrown out (note that it doesn't appear in the Zen of Python -- TOOWTDI is very different).

But let's just do that experiment and see whether we like it. If we do, we can update the PEP, or maybe we'll decide that we'll want a decorator to turn it on (though if you just sometimes want to turn it off, @no_type_check should suffice!).

@JukkaL
Copy link
Contributor

JukkaL commented Aug 26, 2016

This is a case where some tool support, for example in an IDE, would be nice. An IDE could automatically fill in a signature copied from a base class for you, and if you change the signature of a method, it could (optionally) automatically change the signature in all relevant method overrides.

Personally I think that having an explicit annotation improves readability and is worth the extra typing, but I can see how it can feel like redundant work.

@dmoisset
Copy link
Contributor Author

@gvanrossum
My fear is the following: given that object is a common ancestor and already has annotations on typeshed, this would essentially turn PEP 484 into an opt-out (via @no_type_check) instead of opt-in mechanism. Most dunder-methods in non-annotated code will start getting checked when using a tool like mypy.

Perhaps it will work reasonably well but I think it would be hard to fight the political battle to accept this idea.

For me the decorator was also a way to add the explicitness both you and @JukkaL fear that might be lost: when somebody sees a method, there's a local, explicit indication that "the annotations for this are available, just look them up on the base class".

Before trying out annotations I was also inclined towards always having local annotations for the same reasons that you do, but after trying it out it ends up being more confusing (especially for methods with complex signatures); I'd rather think of the API as "this class has the same annotations as a standard UploadHandler" rather than looking at the types of 7 arguments and wondering if they are equal or slightly different to the base class.

For the refactoring case, I think one expects during the refactoring to find problems in method bodies in any inheritance scenario where I change argument types (not just overriding methods but any calls too), so I wouldn't say that it will feel "surprising".

But in short: even if I prefer an explicit way to do it (rather than implicit ;-) ), I still prefer having some version of this rather than none. I'm just trying to convince you to go for an explicit form.

@JukkaL
Copy link
Contributor

JukkaL commented Aug 31, 2016

Let's consider a particular use case. Some user code uses a library or framework that is heavily based on inheritance and method overriding. Now a type checker would potentially type check a large fraction of that code even without any explicit annotations or changes to the code, and this could generate many false positives or warnings. Implicitly inheriting annotations from base classes feels to me to be against the spirit of gradual typing where unannotated code should be accepted silently. Also, when looking at this code, it's unclear which methods will be type checked and which will not, as just looking at a class definition doesn't give enough information to infer which method is an override.

What about a @typing.override decorator instead of inherit_type_hints? Instead of just inheriting the annotations from the base class, it could instead ensure that the method is actually an override and that it has an annotation if the base class method has been annotated. If the override doesn't have an annotation, instead of implicitly inheriting the annotation, it could give a message to make it easier to add the annotation. For example:

from typing import override

class A:
    def f(self, x: int = 1) -> str: ...

class B:
    @override
    def f(self, x=1): ...

Now a type checker could give an error message like this:

Method override should have an annotation since the overridden method in "A" has an annotation
Suggested annotation:

    def f(self, x: int = 2) -> str

That is, the type checker could auto-generate the annotation for the user, at least in typical cases, and applying the annotation would be a simple matter of copy-paste. This would have two benefits over the current state:

  1. We can document method overrides, making code that uses overriding easier to understand. Type checkers could optionally enforce that all overrides are annotated.
  2. Type checkers can generate the annotation for you instead of having to manually copy it. Maybe they could also help by suggesting any missing imports needed for the annotation.

@dmoisset
Copy link
Contributor Author

dmoisset commented Sep 1, 2016

My problem with @JukkaL 's proposal is that even if the API has naturally a single source of truth (the definition on the parent class), that truth has to be duplicated in many places and it's hard to change (call it DRY or other thing if you don't like it, but for me that's as bad engineering as repeating all over the place a literal that should be a constant)

That's why I proposed something like @override but not having to repeat the signature all over again. For me an override is more like an "use" (very liberally speaking) of the API than a definition (and having to annotate those feels like having to annotate function calls in addition to function definitions).

As a side note, it would be nice for this to cover inherited attribute annotations too (now that PEP-526 is coming up). This would actually be nice:

class LibraryBaseClass:
    names = ['Graham', 'Eric', 'Terry', 'John',  'Michael']   # type: List[str]  # actually this is inferred in this case

class UserDefined(LibraryBaseClass):
    names = []  # this could "inherit" the List[str] annotation, doesn't currently, at least in mypy

@gvanrossum
Copy link
Member

gvanrossum commented Sep 1, 2016 via email

@refi64
Copy link

refi64 commented Sep 1, 2016

AFAIK: no. C++ doesn't even have this.

@dmoisset
Copy link
Contributor Author

dmoisset commented Sep 1, 2016

The OO languages I know from the static typing camp (C++, Java, ObjectPascal, Eiffel, Scala) have nothing like this (and I just checked and C# and ObjectiveC don't either). OTOH, most of those don't have a way to avoid specifying argument types in method definitions; Python instead does have that by nature, and currently its meaning is a bit weird wrt to types (overriding method says "unlike the overriden version, I accept anything and might return anything!"). So I admit we're treading new ground here (with gradual typing in general) and may need to do things differently. Or perhaps it's just a stupid idea :)

Digging a bit, one precedent I can find is F#. In this example https://docs.microsoft.com/en-us/dotnet/articles/fsharp/language-reference/members/methods , class Circle has an abstract declaration providing the signature ("annotations", using Python's terms) of a "rotate" method , a default statement with the body (it would be a def in python, but here it's separated from the annotation). Then class Ellipse extends Circle, and explicitly flags as override its new definition (an @override def, using @JukkaL notation) which doesn't specify the signature again.

@dmoisset
Copy link
Contributor Author

dmoisset commented Sep 2, 2016

A more relevant example I found today: TypeScript (which has gradual typing natively) follows my proposal. You can see an example (Animal and Snake classes) at https://www.typescriptlang.org/docs/handbook/classes.html

@JukkaL
Copy link
Contributor

JukkaL commented Sep 2, 2016

I don't think that TypeScript works like that. It seems to infer the argument type from the default value. Consider this example:

class Animal {
    move(distanceInMeters: number) {
        distanceInMeters();   # error
    }
}

class Snake extends Animal {
    move(distanceInMeters) {
        distanceInMeters();  # no error
    }
}

TypeScript also infers the return value type if it's missing, but I doubt that it inherits the declared return value type.

I experimented with this online tool: https://www.typescriptlang.org/play/

@dmoisset
Copy link
Contributor Author

dmoisset commented Sep 2, 2016

Oops, you seem to be right @JukkaL , it seems I misinterpreted what I read.

@sky-code
Copy link

+1 for special type like typing.inherit_type or just typing.inherit

@NeilGirdhar
Copy link

NeilGirdhar commented Mar 13, 2019

I like this idea. I feel like it should be possible to narrow types. In Jukkal's example,

from typing import override

class StringSubclass(str):
    pass

class A:
    def f(self, x: int = 1) -> str:
        ...

class B:
    @override
    def f(self, x=1) -> StringSubclass:
        ...

could somehow be made to inherit x's type, but the return value has been narrowed. Also, it might be nice to be able to remove keyword arguments and trailing positional arguments in overriding methods.

@plcplc
Copy link

plcplc commented Dec 7, 2020

Another way that would preserve some of the explicitness of copying the type hints from the base class is if we could assign type hints to a definition in a way that's syntactically separated from the definition.

Similarly to how for variables we may say:

x : int
x = 42

If we for definitions we could say:

t = a_long_and_complicated_type_i_would_rather_not_keep_repeating
@type(t)
def foo():
    ....

Then the story for "inheriting" method types become much more palatable:

class AbstractFoo(abc.ABC):
    @type(t)
    @abstractmethod
    def foo():
        pass

class ConcreteFoo(AbstractFoo):
   @type(t)
   def foo():
        return 42

With this, declarations of type hints are still explicit and we have gained the ability to talk about type hints for defs the same way we do variables, which seems like a generally useful thing.

However, there is still truth to the issue that error messages that arise when t changes will be talking about errors in the function bodies rather than their stated type signatures. Personally I would feel that that judgement sits better with the programmer than the library designer.

@antonagestam
Copy link

I like @plcplc's idea, as it would be useful for functions as well. When you have a lot of function definitions that should have the same signatures it would be useful to do something like this:

T = Callable[[int], bool]


@annotate(T)
def a(i):
    return i > 0


@annotate(T)
def b(i):
    return i == 2

@skilkis
Copy link

skilkis commented Jul 29, 2021

I ran across this thread as well as #270 which seems somewhat related. Similar to inheriting annotations, from my understanding the other thread was advocating to inherit the arguments themselves through *args and **kwargs. For example, this could be useful to add optional arguments to a specialization class without violating DRY by repeating the function signature. Seeing as #270 is closed now, I'm adding the #270 (comment) to hopefully re-ignite the discussion from @rmorshea who proposed:

class Parent:
    def method(self, y: int, y: int) -> int: ...

class Child(Parent):
    def method(self, z: int, *args: SameAs[Parent]) -> SameAs: ...
    # or
    def method(self, z: int, **kwargs: SameAs[Parent]) -> int: ...
    # or
    def method(self, z: int, *args: SameAs[Parent], **kwargs: SameAs[Parent]) -> int: ...

Also, please correct me if I'm wrong but I don't think that PEP-612 covers the particular use-case of annotation/signature extension. If it does than some examples would be very welcome! I do think that ParamSpec is awesome for decorators by the way 😄

@srittau
Copy link
Collaborator

srittau commented Nov 4, 2021

I believe that this issue is something that individual type checkers can offer as a discretionary feature (possibly using feature flags).

@srittau srittau closed this as completed Nov 4, 2021
@srittau srittau added resolution: out of scope The idea or discussion was out of scope for this repository topic: feature Discussions about new features for Python's type annotations and removed enhancement labels Nov 4, 2021
@zzzeek
Copy link

zzzeek commented Jan 7, 2022

I believe that this issue is something that individual type checkers can offer as a discretionary feature (possibly using feature flags).

I'm possibly coming across this issue right now (will post as a discussion), but how would one apply type annotations to code that is then expected to only pass under certain type checkers that offer that syntax? or am i misunderstanding what this means?

@adam-grant-hendry
Copy link

@JukkaL @dmoisset Hasn't the historically accepted convention when LSP is intended to be broken in Python been to make a from_ method? Would introducing an @override influence users to break this convention?

With DRY, my biggest concern is how easy it is to introduce an accidental error because code is copied or you forget to update another code segment. However, as @gvanrossum pointed out, mypy will tell you if you forgot to add a new overload in your subclass.

I don't often have to go hunting for type definitions in an editor because of Intellisense. The only case I generally do is on GitHub, but I don't find the hunting all that much of a burden (there's often hyperlinks). The same sort of case for readability could be made for many things that have existed in Python for decades: if a subclass method calls self.some_func() that doesn't exist in the subclass, I don't know if it's in the parent, or the parent's parent, or the parent's parent's parent...I'm always hunting in Python.

On the other hand, having to reread the same code I just read over and over again in a code review and scroll past all the signatures just to find the actual business logic is a bit taxing (code is read more than it is written).

I like @plcplc 's idea of shortening the effort of retyping the same thing a good idea. I'm not sure why adding some kind of syntactic sugar (decorator or otherwise) that says "copy of previous stubs" is necessarily a bad thing. Perhaps even putting overload types in .pyi stubs and pointing the same .pyi file could be done.

Could we vote to reopen this issue?

I say +1 for adding a decorator (or something similar)

@carlosgmartin
Copy link

I think it would be useful to have an easy, standardized way to do annotation-inheritance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution: out of scope The idea or discussion was out of scope for this repository topic: feature Discussions about new features for Python's type annotations
Projects
None yet
Development

No branches or pull requests