classification
Title: Instance methods are misreporting the number of arguments
Type: enhancement Stage:
Components: Versions: Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: belopolsky, benjamin.peterson, eric.araujo, giampaolo.rodola, gungorbasa, nedbat, r.david.murray, rbp, rhettinger, tshepang
Priority: low Keywords: patch

Created on 2008-03-30 22:23 by belopolsky, last changed 2013-08-17 15:34 by tshepang.

Files
File name Uploaded Description Edit
issue2516poc.diff belopolsky, 2008-03-31 00:37
issue2516poc1.diff belopolsky, 2008-04-03 19:46 Patch against revision 62106
Messages (13)
msg64767 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-03-30 22:23
Opening a new issue per Raymond's request at msg64764:

"""
It would be *much* more useful to direct effort improving the mis-
reporting of the number of arguments given versus those required for
instance methods:
  >>> a.f(1, 2)
  TypeError: f() takes exactly 1 argument (3 given)
"""

I would suggest that this misreporting may be dear to old-beards 
reminding the time when there was not as much difference between methods 
and functions as there is now.

It does not seem to be that hard to change the diagnostic to 

  >>> a.f(1, 2)
  TypeError: f() takes no arguments (2 given)

but the novice users would much rather see "a.f() takes no arguments (2 
given)." The later is unfortunately not possible.

Raymond, what would you say to "<class 'A' instance>.f() takes no 
arguments (2 given)" diagnostic?
msg64772 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-03-31 00:37
Attached patch (issue2516poc.diff) presents proof-of-concept code which 
changes the problematic reporting as follows:

>>> a.f(1,2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: <A object>.f() takes exactly 0 arguments (2 given)

More effort is needed to make this patch ready: support default/keyword 
arguments, respect English grammar in 1-argument case, etc.  Before I do 
that, however, I would like to hear that this is a worthwhile fix and 
that I've chosen the right place in the code to implement it.
msg64773 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-03-31 01:13
You have +1 from me to continue developing this patch.
msg64902 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-04-03 19:46
I am uploading another work in progress patch because the problem proved
to be more difficult than I thought in the beginning.  The new patch
addresses two issues.

1. a.f(..) -> A.f(a, ..) transformation is performed is several places
in the current code.  I have streamlined CALL_* opcodes processing to
make all calls go through PyObject_Call. This eliminated some
optimizations that can be put back in once the general framework is
accepted.

2. The only solution I could find to fixing reporting from
instancemethod_call was to add expected number of arguments information
to the exception raised in ceval and use it to reraise with a proper
message.  Obviously, putting the necessary info into the argument tuple
 is a hack and prevents reproducing original error messages from the
regular function calls.  I see two alternatives: (a) parsing the error
string to extract the needed information (feels like even a bigger
hack), and (b) creating an ArgumentError subclass of TypeError and have
its instances store needed information in additional attributes (talking
about a canon and a mosquito!)

3. If a solution that requires extra information in an exception is
accepted, PyArg_Parse* functions should be similarly modified to add the
extra info when raising an error.

Finally, let's revisit whether this mosquito deserves to die.  After
all, anyone looking at method definition sees the self argument, so
saying that a.f(1, 2) provides 3 arguments to f() is not such a stretch
of the truth.

It is also possible that I simply fail to see a simpler solution.  It
this case, please enlighten me!

PS: The patch breaks cProfile and several other tests that check error
messages.  I am making it available for discussion only.
msg64903 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-04-03 20:04
Guido, what's your opinion on this? Is this a bug, and should it be fixed?
msg64910 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-04-04 01:39
It's definitely a bug, but I think the reason it has been around so 
long is that no-one has offerred a clean solution.  

I was hoping for something along the lines of functions raising an 
ArgumentError (a new subclass of TypeError) that could be trapped by 
the __call__ slot for bound methods and then reraised with a new 
argument count.  The key is to find a *very* lightweight and minimal 
solution; otherwise, we should just live with it for another 18 years :-
)
msg64932 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-04-04 14:54
On Thu, Apr 3, 2008 at 9:39 PM, Raymond Hettinger
<report@bugs.python.org> wrote:

> It's definitely a bug

What would you say to the following:

def f(x):
    pass
class X:
    xf = f

x = X()
x.xf(1,2)

--> TypeError: f() takes exactly 1 argument (3 given)

Is this correct?

..
>  I was hoping for something along the lines of functions raising an
>  ArgumentError (a new subclass of TypeError) that could be trapped by
>  the __call__ slot for bound methods and then reraised with a new
>  argument count.

This would be my first choice for a clean solution as well.  Since it
will require a change to the exception hierarchy, should we discuss a
modification to PEP 348 on python-dev?   I would rather finish the
patch first and then make a concrete proposal.

> The key is to find a *very* lightweight and minimal solution;

.. according to what metric?  Are you talking about the amount of code
that needs to be changed, the number of API changes or run-time
overhead?  I don't think run-time overhead is an issue: argument
errors are rarely used for flow control  because it is hard to predict
what a failed attempt to call a function would do.
msg108252 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-06-21 00:55
The same subject on python-dev raised last month received positive feedbacks:
http://mail.python.org/pipermail/python-dev/2010-May/100197.html
msg108253 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-06-21 02:41
All that is needed now is a working patch... :)
msg108616 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-25 18:34
Here is a similar issue which may be easier to fix:


>>> def f(a, b=None, *, c=None, d=None):
...    pass


>>> f(1,2,3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: f() takes at most 4 arguments (3 given)


Should be "f() takes at most 2 positional arguments (3 given)"
msg108620 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2010-06-25 19:31
Fixed in r82220.
msg108623 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-06-25 19:42
Maybe I'm misunderstanding what issue we're talking about here, but wasn't this supposed to be fixed?


giampaolo@ubuntu:~/svn/python-3.2$ python3.2
Python 3.2a0 (py3k:82220M, Jun 25 2010, 21:38:56) 
[GCC 4.4.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> class A:
...     def foo(self, x):
...             pass
... 
>>> A().foo()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: foo() takes exactly 2 arguments (1 given)
>>>
msg108632 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-06-25 21:06
I presume Benjamin meant he fixed the special case Alexander reported.
History
Date User Action Args
2013-08-17 15:34:27tshepangsetnosy: + tshepang

versions: + Python 3.4, - Python 3.2
2012-12-11 21:10:59eric.araujosetnosy: + eric.araujo
2010-06-25 21:06:52r.david.murraysetmessages: + msg108632
2010-06-25 19:42:50giampaolo.rodolasetmessages: + msg108623
2010-06-25 19:31:29benjamin.petersonsetmessages: + msg108620
2010-06-25 18:39:49belopolskysetmessages: - msg107175
2010-06-25 18:38:03belopolskysetmessages: - msg91826
2010-06-25 18:37:50belopolskysetmessages: - msg91825
2010-06-25 18:37:42belopolskysetmessages: - msg91824
2010-06-25 18:35:59belopolskysetassignee: belopolsky ->
2010-06-25 18:34:48belopolskysetmessages: + msg108616
2010-06-21 02:42:50benjamin.petersonsetversions: + Python 3.2, - Python 2.6
2010-06-21 02:41:50r.david.murraysetnosy: + r.david.murray
messages: + msg108253
2010-06-21 00:55:22giampaolo.rodolasetnosy: + giampaolo.rodola
messages: + msg108252
2010-06-06 02:20:55belopolskysetassignee: belopolsky
messages: + msg107175
2010-06-06 02:16:07belopolskysetfiles: - unnamed
2010-05-29 02:23:09rbpsetnosy: + rbp
2009-08-21 19:07:38gvanrossumsetnosy: - gvanrossum
2009-08-21 13:39:30gungorbasasetfiles: + unnamed

messages: + msg91826
2009-08-21 13:34:34benjamin.petersonsetfiles: - unnamed
2009-08-21 13:31:05gungorbasasetfiles: + unnamed

messages: + msg91825
2009-08-21 13:17:00benjamin.petersonsettype: compile error -> enhancement
2009-08-21 13:02:07gungorbasasetversions: + Python 2.6
nosy: + gungorbasa

messages: + msg91824

components: - Interpreter Core
type: behavior -> compile error
2008-04-04 14:54:54belopolskysetmessages: + msg64932
2008-04-04 01:39:57rhettingersetmessages: + msg64910
2008-04-03 20:04:23benjamin.petersonsetnosy: + gvanrossum
messages: + msg64903
2008-04-03 19:46:47belopolskysetfiles: + issue2516poc1.diff
messages: + msg64902
2008-03-31 01:13:59benjamin.petersonsetpriority: low
nosy: + benjamin.peterson
messages: + msg64773
2008-03-31 00:37:27belopolskysetfiles: + issue2516poc.diff
keywords: + patch
messages: + msg64772
2008-03-30 23:26:41nedbatsetnosy: + nedbat
2008-03-30 22:23:34belopolskycreate