Pycodestyle: E741 misinterprets function call arguments as variable names

[Julian]

$ virtualenv test && test/bin/pip install flake8==3.5.0 && test/bin/flake8 <(printf 'def foo(l): passnnnfoo(l=12)n'); rm -rf test

produces:

/dev/fd/11:4:5: E741 ambiguous variable name 'l'

l is a function argument there, not a variable.

(I assume the intent was https://www.python.org/dev/peps/pep-0008/#names-to-avoid, but it's not the caller's responsibility what a function argument is called, don't think that's at all what PEP8 means there).

Moved from https://gitlab.com/pycqa/flake8/issues/383#

[IanLee1521]

Hi @Julian --

I believe this is working as expected (and desired). Your code:

def foo(l): pass


foo(l=12)

In line 4 (foo(l=12)) is setting the value 12 to the variable l, which is then being passed in to your function. The error that you're seeing is on that line. This seems to me like what we would want, and if anything I am inclined to think that having the l in the function definition is also not what we want.

[Julian]

@IanLee1521 a function call is not a variable.

Clients do not get to decide what the names of function arguments, so a style checker shouldn't be warning about it there, it's not the call site's responsibility to change the name, and the code might not even be internal.

If you want another example of how silly that'd be, this will warn for a file that contains literally just "{l}".format(l=12).

[IanLee1521]

Actually I think that last case is even more in line with the meat of this rule, namely that there, format is taking a caller defined variable and using it in that string. Instead that should be something like '{length}'.format(length=12) (or some other more descriptive name.

I see where you're coming from, though I'm still not sure I agree. If this is really something that a library is doing, then I personally think that is the bug, and the function definition there should be updated to use a more descriptive name.

[Julian]

I'm not sure what else to say, this seems pretty self-evidently wrong behavior to me, and that you can't go to all the client code and say "file a bug upstream" every time you see a function call, and I don't think this has any grounding in PEP8.

Don't think I can press further then, but if anyone else finds this and happens to agree (or disagree) with me, would love to hear.

[Julian]

@IanLee1521 by the way, completely unrelated to the issue at hand, I just reread my second-to-last message, and like a few of the tickets I've filed today (unfortunately I've filed quite a few), it came off a bit more brisk than it should be, so apologies for that if it did sound that way (filing that many tickets has apparently had that effect on my morning... :/).

[IanLee1521]

I understand.. No worries. :)

If it is something that you're really wanting to clarify you might ask in the python-dev mailing list (I think that is where the conversations typically happen around clarifying the PEPs, but I might be out of date a bit with my info).

I suspect if we were to ask the original PEP-8 authors though, they would probably lean in the direction of making it inclusive of function arguments rather than not. Likely for similar reasons here and keeping with a "spirit" of the rule.

[FichteFoll]

I'm also with @Julian here. Ambiguous argument names are introduced in the function definition, not in the function call. Furthermore, this "error" only has a single source (the definition) whereas all usages of it are caused by that. Regardless of whether you can or cannot change the function's definition, the error should mark the source of the problem and not each of its manifestations.

You can compare this to the current behavior of not highlighting each occurrence of the variable l and instead only the definition.

Regarding the .format case, I'd argue that the same rules as for list comprehensions (and lambda functions?) apply. I seem to remember that single-letter variables are not marked in these instances because the are short-lived, much like format string templates.

[sigmavirus24]

So I'm inclined to agree with @Julian that this check belongs only at the call site. That said, the assertion that this check has no grounding in the PEP is wrong.

With that in mind, I'm working on a patch to fix up E741. I'm most of the way there. I'm presently hacking out how to not warn on function invocations

[FichteFoll]

Quoting the referenced section for convenience:

Never use the characters 'l' (lowercase letter el), 'O' (uppercase letter oh), or 'I' (uppercase letter eye) as single character variable names.

We agreed that this also applies to function arguments, since those are variable declarations, and I'm not sure how you transfer this over to usages of these variables.

(I initially assumed this was about single-letter variables in general, but it's just for special letters. Anyone who uses a font with ambiguous rendering of these variables is doing something wrong in the first place, but that's just my opinion.)

[sigmavirus24]

(I initially assumed this was about single-letter variables in general, but it's just for special letters. Anyone who uses a font with ambiguous rendering of these variables is doing something wrong in the first place, but that's just my opinion.)

If you hate the people reading your code so much, why even use pycodestyle? ;)

[FichteFoll]

It was hard to write, so it should be hard to read :wink:

[dmick]

Yeah, I don't see how it's helpful to drub the user of a badly-designed API. If the API is non-PEP8 compliant, the user can't really do much about it. This is just an automatic "always-disable" warning if you ever get it from a call of code you don't control, which of course subverts the entire purpose of linting.

[sigmavirus24]

Hi @dmick, it sounds like you're frustrated. It'd be more helpful to the project if you could channel that into carrying #707 over the finish line. This is something that has a start, but I haven't had time to finish. If you do, that would eliminate this source of frustration for you and everyone else on this thread.

[dmick]

Just adding votes. The fact is that we've added ignores.

I'm happy you apparently agree; "check belongs only at the call site" seems like a statement of disagreement, and I skimmed, so I was probably misinterpreting your intent.