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: treat "obvious" return type as annotated #4409

Open
elazarg opened this issue Dec 25, 2017 · 42 comments
Open

Proposal: treat "obvious" return type as annotated #4409

elazarg opened this issue Dec 25, 2017 · 42 comments
Labels
feature topic-inference When to infer types or require explicit annotations

Comments

@elazarg
Copy link
Contributor

elazarg commented Dec 25, 2017

(Not sure if it should be here or in typing)

Consider this function:

def f():
    return "hello"
def g(x):
    if x: return A(1)
    else: return A(2)

f obviously (without any deep analysis) returns str, and g returns A. Why not use this information? This pattern is very common, and taking advantage of it can help in precise checking and remove clutter (for example -> Union[Tuple[int, int, str], Tuple[int, str, bool]]).

I propose treating calls to functions whose return expressions consists solely of literals and constructor calls as if they were declared with the returned type (join or union of the return types).

@refi64
Copy link
Contributor

refi64 commented Dec 25, 2017

Well, I always felt that the main reason for this was to allow mypy to be used on already-existing codebases, so the unannotated functions don't need to be type-safe (yet). This would change that behavior.

@ethanhs
Copy link
Collaborator

ethanhs commented Dec 25, 2017

@kirbyfan64 I think I agree with what you are saying, but I also think there isn't a reason something like this couldn't be behind a flag, I've heard from several people that this is a significant pain point, as people don't want to have to do a lot of work to annotate obvious functions, so big +1 from me.

@elazarg
Copy link
Contributor Author

elazarg commented Dec 25, 2017

@kirbyfan64 the proposal does not require unannotated functions to be type-safe. It simply allows annotated functions that use them to be slightly more type safe (peek into the immediate return expressions, if there is one).

@elazarg
Copy link
Contributor Author

elazarg commented Dec 25, 2017

To elaborate:

class A: pass

def f(): return A(1)  # no error here, even though A has no no-arg __init__

def g() -> None:
    f().bar()  # error here

@gvanrossum
Copy link
Member

gvanrossum commented Dec 25, 2017 via email

@elazarg
Copy link
Contributor Author

elazarg commented Dec 25, 2017

Either a literal or a direct constructor call on a globally defined class. (Part of the idea is that the return statements are also the documentation).

@ethanhs
Copy link
Collaborator

ethanhs commented Dec 25, 2017

Expression atoms would be straightforward enough (not certain about identifiers however). At some point this could probably be expanded to entire expressions I believe (down the road).

@elazarg
Copy link
Contributor Author

elazarg commented Dec 25, 2017

Yes, that could be a nice path. Atoms, then (recursively) tuples/lists, then simple names. My hope is this could be done as part of semantic analysis, just like the analysis of the annotations themselves.

@refi64
Copy link
Contributor

refi64 commented Dec 25, 2017

Could a director constructor call really be safe though? e.g.:

def func():
    A = lambda x: x
    return A(None)

Or even:

def func():
    return A(1)

func.__globals__['A'] = lambda x: x

Of course, this isn't ideal, but it happens in unannotated code.

@elazarg
Copy link
Contributor Author

elazarg commented Dec 25, 2017

I think it will be as safe as any other analysis performed by mypy. The first example is not a call to a global constructor; the second example defeats any other analysis.

@elazarg
Copy link
Contributor Author

elazarg commented Dec 26, 2017

I guess the discussion can be separated to two orthogonal questions:

  • What should be considered as "obvious" return type
  • How this mechanism is enabled: a flag for unannotated functions, a flag for partially-annotated functions, using an Infer annotation, or some other means

@ilevkivskyi
Copy link
Member

Note there is an old proposal python/typing#276 to make this explicit. During the discussion it became clear that inferring types for arguments can be very hard, but for return types it is probably OK.

Recently, additional interest in this appeared in the context of data classes (PEP 557), where annotations are syntactically required, but people may be lazy to write exact types, so that this could be allowed:

@dataclass
class C:
    x: ... = 5  # equivalent to 'x = 5' (without an annotation) for a type checker

while for function this will work as

def func() -> ...:
    return 5

reveal_type(func())  # Revealed type is 'builtins.int'

An additional argument in favour of ellipsis is that libraries in numeric stack use ellipsis for inferred number of dimensions.

@elazarg
Copy link
Contributor Author

elazarg commented Dec 27, 2017

One problem with annotations is that they mean that the function will be checked. It also does not completely address the issue of clutter and laziness.

I agree that if this is the direction, then the ellipsis idea is nice. But two minor points should be noted: its meaning is unrelated to Tuple[int, ...], and ... everywhere might make the code look bad.

@ilevkivskyi
Copy link
Member

@elazarg

One problem with annotations is that they mean that the function will be checked.

It is maybe a matter of taste, but I think it is rather a plus, since I like to keep the current separation between annotated and non-annotated functions.

@elazarg
Copy link
Contributor Author

elazarg commented Dec 28, 2017

The idea is to not to make the human hands robotic, but to give them fingernails 😄

Behind a flag, the taste will be the user's.

@ilevkivskyi
Copy link
Member

The problem with a flag is that there already lots of them, so I am not keen to have one more (and this also doesn't solve the @dataclass problem). Anyway, I think we need to hear opinions of others.

@elliott-beach
Copy link
Contributor

I think this definitely should be behind a flag, if included, because it changes the spirit of mypy by not allowing typed functions. There is a similar discussion for typescript, although it's for function arguments rather than return type ( microsoft/TypeScript#15114).

@ethanhs who specifically said the lack of this feature was a pain point (just wondering)?

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 11, 2018

I agree with Guido. It seems that the dividing line will have to be kind of arbitrary.

Somewhat related: One potential longer-term workaround to the problem would be to provide an editor hook that could automatically create a signature for the current function being edited. It could infer simple enough types and leave placeholders for other types. Figuring out the types could be a feature of the mypy daemon mode. Basically this would integrate PyAnnotate-like functionality to an editor.

@elazarg
Copy link
Contributor Author

elazarg commented Jan 11, 2018

@elliott-beach I don't think this is related. I suggest no inference at all, let alone cross-procedural one.

@akdor1154
Copy link

akdor1154 commented Feb 15, 2018

Typescript does this by default and it works beautifully. They have an (encouraged) compiler flag 'noImplicitAny' which will cause an error if the return type cannot be inferred; in this case the dev should manually annotate. The end result is really nice - I get return type safety for free in most cases.

The question about "when is it obvious enough" is pushed under the carpet and answered with "when our static analyzer can understand it". The analyzer improves with most typescript releases, so things that used to require explicit annotation (remember, the user knows this because they will have 'noImplicitAny' turned on) may no longer need them in newer Typescript versions. This has not caused any problems that I am aware of.

It does clash with the current mypy use of type annotations to determine whether to check a function or not. To be honest I reckon this is a pain. I would rather just be able to tell mypy "check this whole file" or if really needed, just use a decorator on specific functions to opt in or out. Other languages (rust, new c++ features, new c# features, swift, ts) are all heavily embracing type inference everywhere, and as a dev I really find this direction to be a lot more ergonomic (safety with less typing [of the keyboard variety], what'd not to like?)

@gvanrossum
Copy link
Member

Maybe we can add a marker to request this, e.g. -> infer. But I presume that even TypeScript doesn't infer the argument types, so this feature seems of limited value except for argument-less functions.

To answer your question "what's not to like", it depends on whether you're working on a small new program or trying to add annotations to millions of lines of legacy code. In the latter case, turning on checking for all code would cause too many false positives (since legacy code tends to use clever hacks that the type checker doesn't understand). If you're in the small-new-program camp, just turn on --check-untyped-defs and other strict flags.

@ilevkivskyi
Copy link
Member

Maybe we can add a marker to request this, e.g. -> infer

and there is an old proposal for this python/typing#276

@ethanhs
Copy link
Collaborator

ethanhs commented Feb 20, 2019

Would it be weird to only do this when --check-untyped-defs is true? Then we can side-step the ambiguity of un-annotated functions and people can opt into this behavior instead of opt out (plus no new flag needed).

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 20, 2019

Would it be weird to only do this when --check-untyped-defs is true?

I like this idea!

@ilevkivskyi
Copy link
Member

I like this too.

@ethanhs
Copy link
Collaborator

ethanhs commented Feb 20, 2019

I like this idea!

I like this too.

Great! Perhaps @elazarg is still interested? If not I might take a shot at it if I can find the time.

@elazarg
Copy link
Contributor Author

elazarg commented Feb 20, 2019

That is a good idea indeed.

I'm still interested (and haven't contributed anything for too long) but I cannot commit to it before mid-April.

@ethanhs
Copy link
Collaborator

ethanhs commented Feb 21, 2019

I'm still interested (and haven't contributed anything for too long) but I cannot commit to it before mid-April.

Glad to hear you are still interested! I think this is definitely a "nice to have" feature so if you find time in the future, great, but no need to get it done soon :)

@JukkaL
Copy link
Collaborator

JukkaL commented Mar 5, 2021

I'm removing the low priority and 'needs discussion' labels since this seems like a popular enough idea. We'd still need somebody to contribute an implementation.

I think that instead of special casing literals and constructor calls, this should use the normal type inference engine (perhaps with some limitations). Otherwise the behavior would be too ad hoc.

For example, these should work:

def f():
    return sys.platform.startswith('win')   # Infer bool as the return type

def g():
    return f()  # Infer bool as the return type

def h():
    return [g(), g()]  # Infer list[bool] as the return type

The type checker already has the concept of 'deferring' type checking of functions if we refer to something without a ready type. In cases like the above, the type of the function would not be ready until we've type checked the body of the function. After type checking we'd infer the return type from the return statements.

The implementation could slow down type inference a lot, so we may need to be more clever about the order of type checking. A minor performance hit would be fine, however.

@DevilXD
Copy link

DevilXD commented Mar 27, 2021

Not sure if this is the best place to suggest this, or should I make a new issue, but I've recently tried using MyPy with the --disallow-incomplete-defs flag on my codebase, and the results I got were quite off-putting, after a bunch of functions that had typed input paremeters, but no -> None return type defined, were returned as problems. While I get the meaning behind the flag (and the results I got), there are cases where a function doesn't even have a return statement inside and isn't used in any expression either (only simple call) - some examples of those are __init__ or helper-setter methods on a class, for example. I find having to add -> None to every function like that quite pointless, as MyPy should be able to detect that easily.

My suggestion would be to infer the return type as None (something that seems to fit within this issue), if the body of the function contains a bare return statement, or no return statement at all. Right now, this is how it looks like:

# mypy: disallow-incomplete-defs

from typing import List

strings: List[str] = []

def add(text: str):  # error: Function is missing a return type annotation
    strings.append(text)

@djmattyg007
Copy link

It really does feel ridiculous that I have to manually add -> None to hundreds of pytest test functions despite them having precisely zero return statements inside of them. Why can mypy not infer this automatically despite it being completely obvious to the human eye? Is there a config setting or command-line flag I can turn on to get this behaviour? I want to turn on strict mode, but I have no interest in adding a totally useless void return type to hundreds of functions before being able to do so.

I even tried looking into what it would take to create a plugin to deal with this problem for pytest, but from what I've been able to find there isn't a plugin hook for this. Nothing seems to be called before checking the type of a standalone function that isn't called by my own code.

@kstauffer
Copy link
Contributor

kstauffer commented Jan 19, 2022

@djmattyg007 worte:

Is there a config setting or command-line flag I can turn on to get this behaviour? I want to turn on strict mode, but I have no interest in adding a totally useless void return type to hundreds of functions before being able to do so.

Create a mypy.ini config file. At the top level of my FancyPackage git repo is mypy.ini:

[mypy]
strict = True
files = FancyPackage

[mypy-FancyPackage._c_wrapper]
ignore_errors = True

[mypy-FancyPackage.test.*]
disallow_untyped_defs = False
disallow_untyped_calls = False

When I run "mypy" with no arguments, the FancyPackage/ subdirectory is checked in strict mode (except for the auto-generated FancyPackage/_c_wrapper.py, a python shim for a C library), but FancyPackage/test/* are only checked for correct usage of FancyPackage without checking the validity of the test functions & classes defined in FancyPackage/test/* per se.

@djmattyg007
Copy link

@kstauffer That doesn't sound like a useful solution at all. It disables a whole bunch of warnings for all affected code, rather than one specific warning for just the functions where it is necessary. All it will lead to is other legitimate warnings being missed.

@kstauffer
Copy link
Contributor

@djmattyg007

That doesn't sound like

(Emphasis added.) Did you actually try it? It disables precisely the warning you don't care about and only in the files where you don't care about it (your test files). My config was just an example. You can tailor yours to your needs. https://mypy.readthedocs.io/en/stable/config_file.html

@djmattyg007
Copy link

djmattyg007 commented Jan 23, 2022

I don't want to disable those settings on a per-file basis. I want to disable them on a per-function basis.

@michaeloliverx
Copy link

TypeScript's inference is really great in this regard, with strict mode enabled you don't have to annotated the return types of functions. I would love some level of support for this in mypy.

@ryanc-bs
Copy link

ryanc-bs commented Nov 8, 2023

It may be obvious to most humans that f should be marked as returning str, but it would be more precise to say it returns Literal["hello"]. Maybe a slightly more complex example could be said to return an enum of possible string literals rather than any arbitrary str. It is also not incorrect to say that f returns Any or object. Automatically inferring the return type could be problematic when the developer intentions are unclear.

@ikokostya
Copy link

@ryanc-bs

It may be obvious to most humans that f should be marked as returning str, but it would be more precise to say it returns Literal["hello"]

The following definitions can be considered as equal:

def f():
    return "hello"

def f():
    x = "hello"
    reveal_type(x) #  Revealed type is "builtins.str"
    return x

mypy already infers type for local variable, and I don't see why rule for inferring return type should be different. For comparison, in both cases pyright infers return type of f as Literal["hello"] (see https://microsoft.github.io/pyright/#/type-inference?id=return-type-inference and https://microsoft.github.io/pyright/#/type-inference?id=literals).

@markshannon
Copy link
Member

Having to write -> None on every function that can only return None because it has no explicit return statements is quite annoying.

In general, if MyPy can infer the return type accurately, then it should. There is no benefit to not doing so.
It doesn't have to report the error, if not in --strict mode, but there seem no reason not to use the information that MyPy already knows.

For example:

def x():
     return 1

def y() -> str:
    return "hello " + x()

The above code is clearly wrong, yet without --strict, x() has the type Any and "hello " + x() isn't checked.
With --strict, MyPy complains that x() is not annotated, which is a distraction from the actual error.

@bluenote10
Copy link
Contributor

bluenote10 commented Dec 1, 2023

In general, if MyPy can infer the return type accurately, then it should. There is no benefit to not doing so.

There probably is a benefit: performance. I'd assume inferring the return types where missing requires a bit of extra work, because currently mypy can just skip checking all returns and their types, which should be faster.

But nonetheless it would be a very nice feature. If performance is a factor it could simply be made an opt-in feature.

@ethanhs
Copy link
Collaborator

ethanhs commented Apr 29, 2024

Ok I think I am going to sit down and commit time to work on an implementation of this. It's a prerequisite to #6646 IMO, and I think fixing that is very important.

I think I will try an implementation with the full inference engine, but I also want to try a more minimal approach, as I think it is less risky.

@Avasam
Copy link
Sponsor Contributor

Avasam commented May 23, 2024

I was gonna ask for some sort of no-untyped-def check (disallow_incomplete_defs or disallow_untyped_defs) that only raises for methods that mypy won't analyze (ie: 0 non-self parameters w/ missing return type, or missing parameter annotations).

But this request would alleviate some of the pain with disallow_incomplete/untyped_defs and reduce a lot of Function is missing a return type annotation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature topic-inference When to infer types or require explicit annotations
Projects
None yet
Development

No branches or pull requests