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

Allow @overload outside stubs #1136

Closed
bdarnell opened this issue Jan 17, 2016 · 17 comments · Fixed by #2603
Closed

Allow @overload outside stubs #1136

bdarnell opened this issue Jan 17, 2016 · 17 comments · Fixed by #2603

Comments

@bdarnell
Copy link
Contributor

Mypy does not consider the stub file for the module it is currently type-checking (and doing so was apparently considered and rejected recently). However, @overload as defined in the PEP is only allowed to appear in stubs, which implies a requirement for type checkers to consider a module's own stubs when type checking it. Otherwise, the ability to type check a module which defines an overloaded function is limited.

My motivating example:

@overload
def utf8(s: None) -> None: ...
@overload
def utf8(s: AnyStr) -> bytes: ...

This could be expressed as def utf8(s: Optional[AnyStr]) -> Optional[bytes], but I'm trying to capture the fact that if the argument is not None then the result is also guaranteed not to be None, so it doesn't introduce unnecessary Optional-ness. (Is there a way to express this with a TypeVar? I don't see one). I can put the overloads in a stub for the benefit of external callers of this module, but then I need to put all the type information for this module in the stub and can't type check it internally (I could duplicate all the non-overload type info in the module itself and get partial coverage). I could also put overloaded functions in their own tiny modules to minimize the impact, but that just feels silly.

@gvanrossum
Copy link
Member

gvanrossum commented Jan 18, 2016 via email

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 18, 2016

I'm sure there's a better way to support @overload than having a separate .pyi file (such as the approach discussed in python/typing#72). Separate .pyi files will be painful to maintain, and they don't provide most of the readability gains of inline annotations.

@bdarnell
Copy link
Contributor Author

I don't see how OptAnyStr would help. Note that there are two different unions involved: (Union[bytes, unicode, None]) -> Union[bytes, None]. I want to constrain the return type as a function of the input type, but it can't just pick the same type on both sides because of the (unicode) -> bytes case.

I'd be happy with a non-stub way to express overloads (and would prefer it to a requirement for separate stub files to use this feature). The # type: comment syntax could support this relatively easily and wouldn't carry the implications of runtime dispatch like a decorator would.

Overloads that can't be satisfied with TypeVars may be rare, but when they come up they can be significant (that's why the feature is in the PEP). In this case, tornado.escape.utf8 is used all over Tornado, and I'd have to add dozens of explicit casts if I had to use the union declaration instead of the overloaded version.

The interaction between stub files and source files is generally awkward. I just realized that the type annotations I've been adding to Tornado source files aren't actually used outside those files because there are stubs for Tornado in typeshed (mypy --verbose should report where it's loading type information from). Assuming I release a version of Tornado with inline type annotations I'm going to need to be able to make sure that those types take precedence over the versions in typeshed.

@gvanrossum
Copy link
Member

gvanrossum commented Jan 22, 2016 via email

@gvanrossum
Copy link
Member

gvanrossum commented Jan 26, 2016 via email

@gvanrossum
Copy link
Member

The PEP is now updated. This issue should stay open until it's been implemented in mypy.

@rwbarton
Copy link
Contributor

What's the idea for how this would actually work in a user program? After all @overload doesn't (and can't I think, especially with Python 2-style type comments) do runtime type-based dispatch.

I think there needs to be just one function body, that mypy will type check with each of the overloaded types. Straw-man syntax in a minimal example:

@overload
def f(x: str) -> str: ...    # literal ...
@overload
def f(x: int) -> int:
    return x

At runtime @overload would be just the identity function and the second definition of f would overwrite the first one. (Maybe for Python 3 @overload could collect the type attributes and store them in a property of the returned function.)

@rwbarton
Copy link
Contributor

Oh interesting, there's actually an example of the expected (but not yet implemented) syntax in the docstring for @overload, which is a bit different.

      @overload
      def utf8(value: None) -> None: ...
      @overload
      def utf8(value: bytes) -> bytes: ...
      @overload
      def utf8(value: str) -> bytes: ...
      def utf8(value):
          # implementation goes here

@gvanrossum
Copy link
Member

Yeah, that's how it should work. The overloads are for the type checker, the final non-overloaded function is for the interpreter. If the latter has annotations it should be checked as a regular function; separately mypy might want to check that it can handle all the overloads; when type-checking a call, only the overloads should be considered.

@cpennington
Copy link
Contributor

cpennington commented Oct 18, 2016

I was looking into implementing this. From what I can see, the example above (utf8) don't typecheck in mypy as of v0.4.5, with the following errors:

$ mypy test_overloads.py
test_overloads.py:4: error: Overloaded function signatures 1 and 2 overlap with incompatible return types
test_overloads.py:4: error: Overloaded function signatures 1 and 3 overlap with incompatible return types
test_overloads.py:9: error: Name 'utf8' already defined

The error on line 9 can be fixed by adding an @overload to the final definition of utf8, but it's not clear to me if that's allowed by the spec or not.

The errors about overlapping signatures are because None inhabits all types, unless --strict-optional is used (and with --strict-optional, the errors go away).

Is there a reason for not putting the implementation in the final @overloaded definition? Also, without --strict-optional, should we ever expect an overload involving None to typecheck?

@gvanrossum
Copy link
Member

gvanrossum commented Oct 18, 2016 via email

@gvanrossum
Copy link
Member

@sixolet Adding you to this issue for coordination purposes.

gvanrossum pushed a commit that referenced this issue Mar 27, 2017
Fixes #1136.

- The implementation must directly follow all the overloads
- The implementation is typechecked exactly according to its own declared types
- Indicates an error if the implementation's argument list is not more general than every override variant
- Also indicates an error if the implementation's return type is also not more general than the return type of every override variant

It also limits overloads to the decorators that are specifically geared towards providing overload-type functionality -- @overload, but also @Property and its cousins @funcname.setter and @funcname.deleter. All other decorators are treated purely as decorators, and now provide redefinition errors if you repeat a function definition with them, instead of errors about an overload you probably did not mean.
This was referenced Apr 21, 2017
@douglas-treadwell
Copy link

douglas-treadwell commented Jun 30, 2017

I don't understand how this solved #2305.

I'm using MyPy 0.511 and still having the issue described in #2305.

@ilevkivskyi
Copy link
Member

It doesn't. Guido was referring to python/typeshed#1136 (on typeshed tracker)

@douglas-treadwell
Copy link

douglas-treadwell commented Jun 30, 2017

Although 0.520-dev (GitHub master) does seem to handle #2305...

Maybe that's because installing MyPy from GitHub master requires preparing the latest typeshed?

@douglas-treadwell
Copy link

@ilevkivskyi can you clarify?

@ilevkivskyi
Copy link
Member

mypy master is synced with typeshed every two-three days. PR python/typeshed#1136 was merged more than two months ago.

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

Successfully merging a pull request may close this issue.

7 participants