Skip to content
This repository has been archived by the owner on Aug 13, 2021. It is now read-only.

Errors about mismatched arguments, especially self #202

Open
alexmojaki opened this issue Apr 10, 2021 · 7 comments
Open

Errors about mismatched arguments, especially self #202

alexmojaki opened this issue Apr 10, 2021 · 7 comments

Comments

@alexmojaki
Copy link

I was looking at this question: https://stackoverflow.com/questions/67034823/trying-to-use-the-return-value-from-one-function-in-another-but-getting-undefine

The poster has made all sorts of errors and their code is generally weird, but I'm just going to focus on the fact that they define a method def add_toys(self,toys) which they try to call with add_toys(self,toys). I put the code into futurecoder to see if it could guide them to a fix. friendly and didyoumean correctly suggest self.add_toys to fix the first exception and self.toys for the next one, e.g:

In your program, add_toys is an unknown name. The local object <Pet object> has an attribute named add_toys. Perhaps you should have written self.add_toys instead of add_toys.

First of all, I think "The local object <Pet object>" could probably be improved a bit to something like "The local variable self with value <Pet object>". But that's a side point.

My main point is that after you follow the suggestions you get self.add_toys(self,self.toys). This gives the error TypeError: add_toys() takes 2 positional arguments but 3 were given. The explanation given is:

You apparently have called the function add_toys with 3 positional argument(s) while it requires 2 such positional argument(s). Perhaps you forgot self when defining add_toys.

That's a good guess from friendly and I understand why it's that way but in this case it's obviously incorrect.

More generally, the whole matter of auto-binding self in methods is very confusing for beginners, errors along these lines are common, and I can't imagine how confusing it must be to be told that they passed 3 positional arguments when they only passed 2 that they know of. Where is that third argument? What on earth is Python talking about?

For all errors about binding arguments, executing can help a lot:

  1. Get the executing node
  2. Check that it's an ast.Call so you don't get an internal error when handling something like def __add__(self, other, foo):
  3. Extract the positional and keyword arguments
  4. Explain what the arguments are to the user, e.g. they passed 2 positional arguments: self and self.toys. I imagine "positional argument" sounds scary and confusing to some.
  5. In particular for this issue, whenever the error message says that the user provided n+1 positional arguments but you only see n, you can guess that self has been provided behind the scenes and explain this to the user. This is more general than the above example - it doesn't matter how many arguments the user provided, how many they were supposed to provide, or what the arguments were (e.g. if they passed self), just that discrepancy between the source code and the message.
  6. Even more generally, if you can use pure_eval to get node.func then you will know what they were trying to call. Then with inspect.signature you can see what arguments they're supposed to provide and explain how Python attempted to bind them. In this case inspect.signature(self.add_toys) is just (toys), while inspect.signature(self.add_toys.__func__) is (self, toys) which might also be useful.
@aroberge
Copy link
Owner

aroberge commented Apr 10, 2021

I was never happy with just guessing "add self", but took it as a decent first step. I'll definitely have to explore using executing, pure_eval and inspect as you suggest.

Note to self: this could be useful https://stackoverflow.com/questions/67639234/count-how-many-arguments-passed-as-positional for further inspiration.

@aroberge
Copy link
Owner

aroberge commented Jul 9, 2021

Without (yet) using executing and friends, I've revised the explanation given by examining the content of the line of code with the error; this is less robust than what you suggest but should cover most examples written by beginners; at the very least, it is an improvement to what was there before.

Here are three different examples that were treated the same way before with the simple suggestion of adding "self".

First example, inspired from a StackOverflow question.

class Pet(object):
    # Inspired by a StackOverflow question
    def __init__(self,name=""):
        self.name = name
        self.toys = []

    def add_toy(self, toy=None):
        if toy is not None:
            self.toys.append(toy)
        return self.toys

    def __str__(self):
        toys_list = add_toy()
        if self.toys:
            return "{} has the following toys: {}".format(self.name, toys_list)
        else:
            return "{} has no toys".format(self.name)

a = Pet('a')
print(a)

Here is the explanation

> python -m friendly_traceback ignore2.py --include why

    In your program, no object with the name `add_toy` exists.

    A local object, `<Pet object> from __main__`,
    has an attribute named `add_toy`.
    Perhaps you should have written `self.add_toy`
    instead of `add_toy`.

A second, slightly different example

class Pet(object):
    # Inspired by a StackOverflow question
    def __init__(self,name=""):
        self.name = name
        self.toys = []

    def add_toy(self, toy=None):
        if toy is not None:
            self.toys.append(toy)
        return self.toys

    def __str__(self):
        toys_list = add_toy(self, 'something')
        if self.toys:
            return "{} has the following toys: {}".format(self.name, toys_list)
        else:
            return "{} has no toys".format(self.name)

a = Pet('a')
print(a)

and the explanation

> python -m friendly_traceback ignore2.py --include why

    In your program, no object with the name `add_toy` exists.

    The local object `<Pet object> from __main__`
    has an attribute named `add_toy`.
    Perhaps you should have written `self.add_toy(...`
    instead of `add_toy(self, ...`.

Finally, a very different example where, once again, the previous explanation was to add self ... and made little sense.

>>> import turtle
>>> p = Pen()

Traceback (most recent call last):
  File "<friendly-console:2>", line 1, in <module>
    p = Pen()
NameError: name 'Pen' is not defined

        Did you mean `len`?
        Did you forget to add `turtle.`?
>>> why()

    In your program, no object with the name `Pen` exists.
    The Python builtin `len` has a similar name.

    The local object `turtle`
    has an attribute named `Pen`.
    Perhaps you should have written `turtle.Pen`
    instead of `Pen`.

@aroberge
Copy link
Owner

aroberge commented Jul 9, 2021

@alexmojaki It it be possible with executing and friends to get the text of a complete statement in which a node is found?

@alexmojaki
Copy link
Author

The Executing object has an attribute statements which is a set of statement nodes, usually just 1.

For an arbitrary node, i.e. not necessarily the executing node, there's statement_containing_node.

For the text of any node, use source.asttokens().get_text(self.node).

You may not actually want the full text if it's a compound statement which could easily be hundreds of lines long, in which case you want a stack_data piece.

@aroberge
Copy link
Owner

aroberge commented Jul 9, 2021

I couldn't figure out what you meant by using a stack_data piece.

This is what I have now, which works with my current tests:
https://github.com/friendly-traceback/friendly-traceback/blob/main/friendly_traceback/utils.py#L166

The purpose is to be able to capture the self appearing on line 190 https://github.com/friendly-traceback/friendly-traceback/blob/main/tests/runtime/test_name_error.py#L189
so that the proper error message can be given.

@alexmojaki
Copy link
Author

See the README at https://github.com/alexmojaki/stack_data

A piece is a range of one or more lines in a file that should logically be grouped together. A piece contains either a single simple statement or a part of a compound statement (loops, if, try/except, etc) that doesn't contain any other statements. Most pieces are a single line, but a multi-line statement or if condition is a single piece.

If the node of interest is contained in the condition of an if statement, you likely just want to display the if <condition>: part, not the body and beyond.

You mentioned pieces in #139 so I think you've seen them before.

FrameInfo.executing_piece will be a range of line numbers containing the line that failed.

If you still want the full statement, you should probably use Source.statements_at_line since you might not have the node, although you will need to account for multiple statements in case there are semicolons.

@aroberge
Copy link
Owner

Since this issue was filed, I made a few changes which I believe do provide suggestions that would not be misleading (in that they would not yield a TypeError: add_toys() takes 2 positional arguments but 3 were given exception. Two different cases can be seen starting at https://friendly-traceback.github.io/docs/tracebacks_en_3.9.html#missing-self-1.

I'm starting to migrate relevant issues from this repository to the newer ones, before freezing this repo. I am thinking that I can close this specific issue but will wait for some feedback.

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

No branches or pull requests

2 participants