Adding more information to "Too Few Arguments" #9796

abhinaypandey02 · 2020-12-12T11:46:36Z

Description

As mentioned in the feature request, mypy now gives more information about the missing required arguments when creating an instance.
The problem was with the if statement that only gave information when the arguments provided were not empty and none. Removing that check gives information for calls with no argument too.

Before:
test.py:6: error: Too few arguments for "Foo"

After:
test.py:7: error: "Foo" missing 1 required positional argument: 'bar'

Test Plan

All tests passing

abhinaypandey02 · 2020-12-12T12:39:39Z

I am sorry I don't have much knowledge about tests. Do I need to manually change all the tests because "Too few arguments" error is invoked plenty of times. Forgive me for sounding stupid I am new.

also checking for None

TH3CHARLie · 2020-12-12T12:53:01Z

Do I need to manually change all the tests because "Too few arguments" error is invoked plenty of times.

Yes but you can do it smartly. Using --update-data flag can save a lot of trouble, see: https://github.com/python/mypy/blob/master/mypyc/doc/dev-intro.md#ir-tests

TH3CHARLie · 2020-12-13T08:53:44Z

And you also need to fix your code formatting to pass flake8 tests

abhinaypandey02 · 2020-12-13T09:19:42Z

Do I need to manually change all the tests because "Too few arguments" error is invoked plenty of times.

Yes but you can do it smartly. Using --update-data flag can save a lot of trouble

None of pytest --update-data mypy, pytest --update-data, pytest --update-data mypyc worked for me when executed in the mypyc/ main directory.They all just add a newline above the test that fails. Am I missing something?(Eventually I built my own python script to modify test files)

Also I integrated flake8 with my Visual Studio code. It would be nice if you could help me figure out the flake8 arguments/settings used with mypy (other than max-length, I am getting many errors that are ignored by mypy's flake8, so what all am I supposed to add to --ignore argument.

TH3CHARLie · 2020-12-13T09:26:23Z

To submit a PR to mypy, all you need is to run runtests.py in your workspace. The script would run self type checking, formatting checking, and IR tests. There's no need to write your own custom script or use vscode's flake8 functionality.

For the --update-data flag, add the flag to runtests.py when you actually need it and remember to exclude this change when you are committing the changes.

This may help you have a better understanding of mypy testing workflow: https://github.com/python/mypy/blob/master/test-data/unit/README.md

abhinaypandey02 · 2020-12-13T09:27:28Z

To submit a PR to mypy, all you need is to run runtests.py in your workspace. The script would run self type checking, formatting checking, and IR tests. There's no need to write your own custom script or use vscode's flake8 functionality.

For the --update-data flag, add the flag to runtests.py when you actually need it and remember to exclude this change when you are committing the changes.

This may help you have a better understanding of mypy testing workflow: https://github.com/python/mypy/blob/master/test-data/unit/README.md

Thanks a lot and sorry for not giving the readme a read.

TH3CHARLie

The change to existing code needs some second thoughts, see comments below.

TH3CHARLie · 2020-12-13T12:56:53Z

test-data/unit/check-callable.test

@@ -110,7 +110,7 @@ from typing import Callable, Union
 x = 5  # type: Union[int, Callable[[], None], Callable[[int], None]]

 if callable(x):
-    x()  # E: Too few arguments
+    x()  # E: Missing positional argument


This error message should not apply to Callable IMO. The current one would do the job.

TH3CHARLie · 2020-12-13T13:16:13Z

test-data/unit/check-classes.test

@@ -3751,7 +3751,7 @@ int.__eq__(int)
 int.__eq__(3, 4)
 [builtins fixtures/args.pyi]
 [out]
-main:33: error: Too few arguments for "__eq__" of "int"
+main:33: error: Missing positional arguments 


I understand callable and lamda functions. But here are you pointing at magic methods?

If I make my message something as Missing positional argument for call to "__eq__" of "int" then the only difference would be language. So is it about language or the missing function name here?

I think the original language is sufficient. The new one does not make much improvement.

So I think where the information about the arguments to be provided is not available, i.e. the message is just "Missing positional argument", there we can leave it to the old language? Should I code it that way?

exactly. For callables, lambdas, magic methods (and maybe more), the names of the arguments are unknown so staying to the old language is totally fine. Let's improve the language only for user-defined functions and methods in this PR. If there's a need to improve other cases in the future, we can do them later in separate PRs.

TH3CHARLie · 2020-12-13T13:17:14Z

test-data/unit/check-inference-context.test

@@ -623,7 +623,7 @@ reveal_type((lambda x, y: x + y)(1, 2))  # N: Revealed type is 'builtins.int'
 reveal_type((lambda s, i: s)(i=0, s='x')) # N: Revealed type is 'Literal['x']?'
 reveal_type((lambda s, i: i)(i=0, s='x')) # N: Revealed type is 'Literal[0]?'
 reveal_type((lambda x, s, i: x)(1.0, i=0, s='x')) # N: Revealed type is 'builtins.float'
-(lambda x, s, i: x)() # E: Too few arguments
+(lambda x, s, i: x)() # E: Missing positional arguments


same for lambda functions

TH3CHARLie · 2020-12-13T15:50:19Z

test-data/unit/check-errorcodes.test

@@ -214,7 +214,7 @@ main:5: error: Invalid "type: ignore" comment  [syntax]

 [case testErrorCodeArgKindAndCount]
 def f(x: int) -> None: pass  # N: "f" defined here
-f()  # E: Too few arguments for "f"  [call-arg]
+f()  # E: Missing positional argument "x" in call to "f"  [call-arg]


this is a perfect example of what's expected in this PR IMO

TH3CHARLie · 2020-12-14T11:11:31Z

Thanks for the updates! Can you verify the case in #9340? For example, use the following code(copied from the issue):

class Foo:
    def __init__(self, bar: int):
        pass

c = Foo()

def foo(baz: int, bas: int):
    pass

foo()

You may also make this a new test case. I did some verification locally and looks like the changes don't work on this segment.

abhinaypandey02 · 2020-12-14T12:42:06Z

@TH3CHARLie I have tested the code in the issue #9340 and the new changes reflect. I made a new test case to check missing positional arguments using the code in the issue and it passes.

TH3CHARLie

Thanks for the updates and verifications, looks good to me. However I am not sure I am 100% in sync with the expectations in #9340, @gvanrossum Would you mind taking a look if you have time?

abhinaypandey02 · 2020-12-15T15:09:42Z

Thanks for the updates and verifications, looks good to me. However I am not sure I am 100% in sync with the expectations in #9340, @gvanrossum Would you mind taking a look if you have time?

Thanks a lot for your review and approval.

gvanrossum

Hm, the number of tests that have to be fixed is a bit much, and in some cases the name of the missing argument isn't very useful, but I agree its most consistent. So weak +1.

TH3CHARLie · 2020-12-17T08:39:29Z

Hm, the number of tests that have to be fixed is a bit much, and in some cases the name of the missing argument isn't very useful, but I agree its most consistent. So weak +1.

Thanks for the review. I am merging this now but I'll keep an eye on this(and respond to any future feedbacks about it) since error messages are important and pervasive

Adding more information to "Too Few Arguments"

5ec4591

abhinaypandey02 closed this Dec 12, 2020

abhinaypandey02 reopened this Dec 12, 2020

using callable_name(callee) instead of callee.name

da3f232

also checking for None

fixed tests

00e7962

fixed flake8 tests

9b332ef

fixed pre-written code instead of writing my own

68a6029

TH3CHARLie reviewed Dec 13, 2020

View reviewed changes

back to old lang when no args info available

604a885

adding test case for verifying the issue

0e3308f

TH3CHARLie approved these changes Dec 15, 2020

View reviewed changes

gvanrossum approved these changes Dec 15, 2020

View reviewed changes

TH3CHARLie merged commit eb0bf11 into python:master Dec 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding more information to "Too Few Arguments" #9796

Adding more information to "Too Few Arguments" #9796

abhinaypandey02 commented Dec 12, 2020 •

edited

abhinaypandey02 commented Dec 12, 2020

TH3CHARLie commented Dec 12, 2020

TH3CHARLie commented Dec 13, 2020

abhinaypandey02 commented Dec 13, 2020 •

edited

TH3CHARLie commented Dec 13, 2020

abhinaypandey02 commented Dec 13, 2020

TH3CHARLie left a comment

TH3CHARLie Dec 13, 2020

TH3CHARLie Dec 13, 2020

abhinaypandey02 Dec 13, 2020

abhinaypandey02 Dec 13, 2020

TH3CHARLie Dec 13, 2020

abhinaypandey02 Dec 13, 2020

TH3CHARLie Dec 13, 2020

TH3CHARLie Dec 13, 2020

TH3CHARLie Dec 13, 2020

TH3CHARLie commented Dec 14, 2020

abhinaypandey02 commented Dec 14, 2020

TH3CHARLie left a comment

abhinaypandey02 commented Dec 15, 2020

gvanrossum left a comment

TH3CHARLie commented Dec 17, 2020

Adding more information to "Too Few Arguments" #9796

Adding more information to "Too Few Arguments" #9796

Conversation

abhinaypandey02 commented Dec 12, 2020 • edited

Description

Test Plan

abhinaypandey02 commented Dec 12, 2020

TH3CHARLie commented Dec 12, 2020

TH3CHARLie commented Dec 13, 2020

abhinaypandey02 commented Dec 13, 2020 • edited

TH3CHARLie commented Dec 13, 2020

abhinaypandey02 commented Dec 13, 2020

TH3CHARLie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TH3CHARLie commented Dec 14, 2020

abhinaypandey02 commented Dec 14, 2020

TH3CHARLie left a comment

Choose a reason for hiding this comment

abhinaypandey02 commented Dec 15, 2020

gvanrossum left a comment

Choose a reason for hiding this comment

TH3CHARLie commented Dec 17, 2020

abhinaypandey02 commented Dec 12, 2020 •

edited

abhinaypandey02 commented Dec 13, 2020 •

edited