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

Confusing/incorrect error message when using max(iterator, key = <func>) on an iterator of NamedTuples #1982

Closed
ldorigo opened this issue Oct 26, 2021 · 5 comments
Labels
enhancement New feature or request fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@ldorigo
Copy link

ldorigo commented Oct 26, 2021

Issue Type: Bug

Minimal code to reproduce:

Test = NamedTuple(
    "Test",
    [
        ("val", Optional[int]),
    ],
)


def foo(a: Dict[str, List[Test]]) -> None:
    for key, vals in a.items():
        t = max(vals, key=lambda x: x.val)

After a while (my code is much more complex), I figured out that the problem is that Test.val might be None in which case comparison fails - but Pylance says "No overloads for "max" match the provided arguments Argument types: (List[Test], (x: Unknown) -> Unknown)". But here the type of x should be known, and the error should be something like "unsupported operation for type int | None".

Extension version: 2021.10.3-pre.1
VS Code version: Code 1.61.1 (c13f1abb110fc756f9b3a6f16670df9cd9d4cf63, 2021-10-14T01:15:35.620Z)
OS version: Linux x64 5.14.6-zen1-1-zen
Restricted Mode: No
Remote OS version: Linux x64 5.10.72-1-lts

System Info
Item Value
CPUs AMD Ryzen 7 4800H with Radeon Graphics (16 x 1400)
GPU Status 2d_canvas: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
opengl: enabled_on
rasterization: disabled_software
skia_renderer: enabled_on
video_decode: disabled_software
vulkan: disabled_off
webgl: enabled
webgl2: enabled
Load (avg) 3, 2, 2
Memory (System) 30.78GB (0.48GB free)
Process Argv --crash-reporter-id 9614dc43-c276-4d0e-94fc-c84de505d77f
Screen Reader no
VM 0%
DESKTOP_SESSION plasma-i3
XDG_CURRENT_DESKTOP KDE
XDG_SESSION_DESKTOP KDE-i3
XDG_SESSION_TYPE x11
Item Value
Remote SSH: puf
OS Linux x64 5.10.72-1-lts
CPUs AMD Ryzen 7 3700X 8-Core Processor (16 x 4198)
Memory (System) 31.36GB (1.09GB free)
VM 0%
@erictraut
Copy link
Contributor

Thanks for the enhancement request. I agree that the error message for a "no overloads available" condition is not as clear as it could be. I'll look at ways to improve it. I've created a tracking bug in the pyright issue tracker.

@judej judej added the needs investigation Could be an issue - needs investigation label Oct 26, 2021
@github-actions github-actions bot removed the triage label Oct 26, 2021
@erictraut
Copy link
Contributor

erictraut commented Oct 30, 2021

I made the following changes that improve the readability of error messages for call expressions that involve overloads.

Previously, if an argument list didn't match any overload for any reason, pylance would emit an error message No overloads for {function} match the provided arguments., and the error was associated with the entire call expression.

The new logic does the following:

  1. It first attempts to narrow the list of overloads based on arity rules. For example, if there are two arguments and only one overload accepts two arguments, all of the other overloads are immediately eliminated from consideration.
  2. If the first step eliminated all possible overloads, the older error message is used.
  3. If the first step eliminated all but one overload, the normal type-matching rules are then applied to that one overload, just as if there was no overload. This provides rich error information about specific arguments/parameters and associates them with the individual argument subexpressions.
  4. If the first step left two or more overloads, the normal type-matching rules are then applied to those overloads. This potentially includes expanding union types for arguments and applying these expanded argument lists to two or more overloads.
  5. If step 4 fails to find an overload (or multiple overloads in the case of union arguments) that satisfy type constraints, the normal type matching rules are applied once more for the last overload that was generated in the first step. As with case 3, this results in rich error information.

In the code sample above, the message was previously:

No overloads for "max" match the provided arguments
  Argument types: (List[Test], (x: Unknown) -> Unknown)

It is now much more understandable:

Argument of type "(x: Test) -> int | None" cannot be assigned to parameter "key" of type "(_T@max) -> SupportsLessThan" in function "max"
  Type "(x: Test) -> int | None" cannot be assigned to type "(_T@max) -> SupportsLessThan"
    Function return type "int | None" is incompatible with type "SupportsLessThan"
      Type "int | None" cannot be assigned to type "SupportsLessThan"
        "__lt__" is not present

@erictraut erictraut added enhancement New feature or request fixed in next version (main) A fix has been implemented and will appear in an upcoming version and removed needs investigation Could be an issue - needs investigation labels Oct 30, 2021
@ldorigo
Copy link
Author

ldorigo commented Nov 2, 2021

It is not a much more understandable:

I assume you meant "it is now" ? I definitely think it's a huge improvement - thanks a lot for this.

@erictraut
Copy link
Contributor

Yes, I meant "now". I fixed my typo above for clarity.

@heejaechang
Copy link
Contributor

fixed in 2021.11.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed in next version (main) A fix has been implemented and will appear in an upcoming version
Projects
None yet
Development

No branches or pull requests

4 participants