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

Pylance incorrectly marks default parameters as a type issue #2879

Closed
lee-hyunkyu opened this issue Jun 1, 2022 · 10 comments
Closed

Pylance incorrectly marks default parameters as a type issue #2879

lee-hyunkyu opened this issue Jun 1, 2022 · 10 comments

Comments

@lee-hyunkyu
Copy link

Environment data

  • Language Server version: 2022.5.3 (pyright 73c82fab)
  • OS and version: macOS 12.2.1
  • Python version (& distribution if applicable, e.g. Anaconda): 3.9

Code Snippet

def foo(a: str = None):
    ...

Expected behavior

I expect this to not throw an type check error and a to be typed as Optional[str] within foo.

Actual behavior

a is typed as str and this raises a reportGeneralTypeIssues error

Logs

[Info  - 10:14:21 AM] (29216) Pylance language server 2022.5.3 (pyright 73c82fab) starting
[Info  - 10:14:21 AM] (29216) Server root directory: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist
Notebook support: Legacy
[Info  - 10:14:21 AM] (29216) No pyproject.toml file found.
[Info  - 10:14:21 AM] (29216) Setting pythonPath for service "<default>": "/Users/hyunkyulee/.pyenv/versions/3.9.11/bin/python"
[Warn  - 10:14:21 AM] (29216) stubPath typings is not a valid directory.
[Info  - 10:14:21 AM] (29216) Assuming Python version 3.9
[Info  - 10:14:21 AM] (29216) Assuming Python platform Darwin
[Info  - 10:14:21 AM] (29216) Search paths for <default>
[Info  - 10:14:21 AM] (29216)   /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib
[Info  - 10:14:21 AM] (29216)   typings
[Info  - 10:14:21 AM] (29216)   /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stubs/...
[Info  - 10:14:21 AM] (29216)   /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/bundled/stubs
[Info  - 10:14:21 AM] (29216)   /Users/hyunkyulee/.pyenv/versions/3.9.11/lib/python3.9
[Info  - 10:14:21 AM] (29216)   /Users/hyunkyulee/.pyenv/versions/3.9.11/lib/python3.9/lib-dynload
[Info  - 10:14:21 AM] (29216)   /Users/hyunkyulee/.pyenv/versions/3.9.11/lib/python3.9/site-packages
[Info  - 10:14:21 AM] (29216) Adding fs watcher for library directories:
 /Users/hyunkyulee/.pyenv/versions/3.9.11/lib/python3.9
/Users/hyunkyulee/.pyenv/versions/3.9.11/lib/python3.9/lib-dynload
/Users/hyunkyulee/.pyenv/versions/3.9.11/lib/python3.9/site-packages
[Info  - 10:14:21 AM] (29216) Searching for source files
[Info  - 10:14:21 AM] (29216) No source files found.
(29216) [IDX(FG)] index libraries  (index) ...
(29216) [IDX(FG)]   read stdlib indices (12ms)
(29216) [IDX(FG)] index libraries  (index) [succeed] (12ms)
(29216) [FG] parsing: /Users/hyunkyulee/Developer/test.py (7ms)
(29216) [FG] parsing: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib/builtins.pyi [fs read 2ms] (48ms)
(29216) [FG] binding: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib/builtins.pyi (13ms)
(29216) [FG] binding: /Users/hyunkyulee/Developer/test.py (0ms)
[Info  - 10:14:21 AM] (29216) No pyproject.toml file found.
[Info  - 10:14:21 AM] (29216) Setting pythonPath for service "<default>": "/Users/hyunkyulee/.pyenv/versions/3.9.11/bin/python"
[Warn  - 10:14:21 AM] (29216) stubPath typings is not a valid directory.
[Info  - 10:14:21 AM] (29216) Assuming Python version 3.9
[Info  - 10:14:21 AM] (29216) Assuming Python platform Darwin
[Info  - 10:14:21 AM] (29216) Search paths for <default>
[Info  - 10:14:21 AM] (29216)   /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib
[Info  - 10:14:21 AM] (29216)   typings
[Info  - 10:14:21 AM] (29216)   /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stubs/...
[Info  - 10:14:21 AM] (29216)   /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/bundled/stubs
[Info  - 10:14:21 AM] (29216)   /Users/hyunkyulee/.pyenv/versions/3.9.11/lib/python3.9
[Info  - 10:14:21 AM] (29216)   /Users/hyunkyulee/.pyenv/versions/3.9.11/lib/python3.9/lib-dynload
[Info  - 10:14:21 AM] (29216)   /Users/hyunkyulee/.pyenv/versions/3.9.11/lib/python3.9/site-packages
[Info  - 10:14:21 AM] (29216) Adding fs watcher for library directories:
 /Users/hyunkyulee/.pyenv/versions/3.9.11/lib/python3.9
/Users/hyunkyulee/.pyenv/versions/3.9.11/lib/python3.9/lib-dynload
/Users/hyunkyulee/.pyenv/versions/3.9.11/lib/python3.9/site-packages
[Info  - 10:14:21 AM] (29216) Searching for source files
[Info  - 10:14:21 AM] (29216) No source files found.
(29216) [IDX(FG)] index libraries  (index) ...
(29216) [IDX(FG)]   read stdlib indices (12ms)
(29216) [IDX(FG)] index libraries  (index) [succeed] (12ms)
[Info  - 10:14:21 AM] (29216) Background analysis(1) root directory: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist
[Info  - 10:14:21 AM] (29216) Background analysis(1) started
(29216) Background analysis message: setConfigOptions
(29216) Background analysis message: setImportResolver
(29216) Background analysis message: ensurePartialStubPackages
(29216) [FG] parsing: /Users/hyunkyulee/Developer/test.py (3ms)
(29216) [FG] parsing: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib/builtins.pyi [fs read 1ms] (29ms)
(29216) [FG] binding: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib/builtins.pyi (7ms)
(29216) [FG] binding: /Users/hyunkyulee/Developer/test.py (0ms)
(29216) Background analysis message: setTrackedFiles
(29216) Background analysis message: markAllFilesDirty
(29216) Background analysis message: setFileOpened
(29216) Background analysis message: getDiagnosticsForRange
(29216) Background analysis message: setConfigOptions
(29216) Background analysis message: setImportResolver
(29216) Background analysis message: ensurePartialStubPackages
(29216) Background analysis message: setTrackedFiles
(29216) Background analysis message: markAllFilesDirty
(29216) Background analysis message: getSemanticTokens full
(29216) [BG(1)] getSemanticTokens full at /Users/hyunkyulee/Developer/test.py ...
(29216) [BG(1)]   parsing: /Users/hyunkyulee/Developer/test.py (7ms)
(29216) [BG(1)]   parsing: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib/builtins.pyi [fs read 2ms] (47ms)
(29216) [BG(1)]   binding: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib/builtins.pyi (15ms)
(29216) [BG(1)]   binding: /Users/hyunkyulee/Developer/test.py (0ms)
(29216) [BG(1)]   parsing: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib/typing_extensions.pyi [fs read 1ms] (4ms)
(29216) [BG(1)]   binding: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib/typing_extensions.pyi (1ms)
(29216) [BG(1)]   parsing: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib/typing.pyi [fs read 0ms] (11ms)
(29216) [BG(1)]   binding: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib/typing.pyi (4ms)
(29216) [BG(1)]   parsing: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib/_typeshed/__init__.pyi [fs read 1ms] (8ms)
(29216) [BG(1)]   binding: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib/_typeshed/__init__.pyi (1ms)
(29216) [BG(1)]   parsing: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib/abc.pyi [fs read 0ms] (0ms)
(29216) [BG(1)]   binding: /Users/hyunkyulee/.vscode/extensions/ms-python.vscode-pylance-2022.5.3/dist/typeshed-fallback/stdlib/abc.pyi (0ms)
(29216) [BG(1)] getSemanticTokens full at /Users/hyunkyulee/Developer/test.py (112ms)
(29216) Background analysis message: analyze
(29216) [BG(1)] analyzing: /Users/hyunkyulee/Developer/test.py ...
(29216) [BG(1)]   checking: /Users/hyunkyulee/Developer/test.py (2ms)
(29216) [BG(1)] analyzing: /Users/hyunkyulee/Developer/test.py (2ms)
(29216) Background analysis message: getDiagnosticsForRange
(29216) Background analysis message: resumeAnalysis
(29216) Background analysis message: getDiagnosticsForRange
(29216) Background analysis message: getDiagnosticsForRange
@github-actions github-actions bot added the triage label Jun 1, 2022
@bschnurr
Copy link
Member

bschnurr commented Jun 1, 2022

You still need to add None as a type.

python 3.10

def foo(a: str | None = None):
    ...

else

from typing import Optional
def foo(a: Optional[str] = None):
    ...

@judej judej closed this as completed Jun 1, 2022
@lee-hyunkyu
Copy link
Author

lee-hyunkyu commented Jun 1, 2022

I want to clarify that the above is not sufficient. By typing it explicitly as str | None, the following usage of foo becomes possible.

t = foo(a=None)

However I want to explicitly disallow this such that the user can ONLY pass in value of type str

@judej judej reopened this Jun 1, 2022
@erictraut
Copy link
Contributor

If the function is written to accept a None value for this parameter, why would you want to prevent a caller from passing None as an argument? There's no way to specify that in the type system because it's a contradiction.

@debonte
Copy link
Contributor

debonte commented Jun 1, 2022

@lee-hyunkyu, is your goal to be able to tell whether the caller (explicitly) provided a value for that parameter?

@lee-hyunkyu
Copy link
Author

@erictraut This is a flow that is supported by mypy (which is why I was quite surprised when pylance did not) so even if it's a contradiction I don't think it's impossible to specify via the type system.

why would you want to prevent a caller from passing None as an argument?

As @debonte mentioned, one reason might be to determine whether the caller explicitly provided a value or not.

Another is that it's a bit of a code smell for a user to use the function above by passing in None explicitly. The user is now coding around the implementation rather than the specification (i.e. they know that passing in None produces some desired behavior, in this case the default behavior, but if in the future I decide to change my implementation such that passing in None produces a different behavior while maintaining the default behavior, the user will need to update their code). Consider the following (rather contrived) example

# v1
def foo(a: str = None):
    if a is None:
        return "foo"
    else:
        return f"foo {a}"

# v2
def foo(a: Optional[str] = 1):
    # a should be of type `str | int | None`
    if type(a, int):
        return "foo"
    else if a is None:
        return "foo None"
    else:
         return f"foo {a}"

## usage

# v1
foo(a=None) # "foo"
foo() # "foo"

# v2
foo(a=None) # "foo None"
foo() # "foo"

The default behavior has remained the same (we return "foo") but when we pass in None, the behavior is different. However for the user of the function, the behavior they wanted isn't different (they want the default behavior which is for it to return "foo") but because they passed in None and the implementation of how it handles None has changed, they now need to go back and update their code. We can instead prevent that altogether by preventing them from ever passing in None. It gives me, the person creating common libraries/functions, more confidence that I can make a "breaking" change without affecting production code since the behavior I'm modifying isn't in use anywhere.

@lee-hyunkyu
Copy link
Author

@debonte In case my previous comment didn't answer your question, no my goal isn't to tell whether a value was explicitly provided. I can't think of a scenario where that would matter but someone else might have a use for that functionality

@erictraut
Copy link
Contributor

erictraut commented Jun 1, 2022

@lee-hyunkyu, mypy implicitly interprets a parameter with a None default value as Optional. In other words, it assumes that a in your example above has the type Optional[str]. That means callers can pass None as an argument, and mypy will not complain. To confirm this, try the following in mypy:

def foo(a: str = None):
    ...

foo(a=None)

This implicit interpretation of Optional is no longer recommended for type checkers. PEP 484 was updated about two years ago, but mypy has not yet been updated to conform with the updated standard. Here is the open issue in the mypy bug database that is tracking that issue.

Here's the relevant section of PEP 484:

A past version of this PEP allowed type checkers to assume an optional type when the default value is None, as in this code:

def handle_employee(e: Employee = None): ...
This would have been treated as equivalent to:

def handle_employee(e: Optional[Employee] = None) -> None: ...
This is no longer the recommended behavior. Type checkers should move towards requiring the optional type to be made explicit.

If you want the old behavior, pyright has an option to enable it. Look for strictParameterNoneValue in this documentation. (Pyright is the type checker that pylance is built upon.)

@lee-hyunkyu
Copy link
Author

@erictraut Hmm, I seemed to have completely misunderstood mypy's behavior, I had thought mypy was catching this but clearly not.

Given that and the PEPs, I'll close. Thanks for the clarification.

@erictraut
Copy link
Contributor

@lee-hyunkyu, if you want to enforce that callers never pass None explicitly, you can use an overload, as follows:

@overload
def foo() -> str: ...

@overload
def foo(a: str) -> str: ...

def foo(a: str | None = None):
    if a is None:
        return "foo"
    else:
        return f"foo {a}"

@lee-hyunkyu
Copy link
Author

@erictraut That's incredibly helpful! I hadn't thought to use overloads to achieve the behavior I want.

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

No branches or pull requests

5 participants