Ignore "_" vars in "Unused Variable" check

Bug #1328169 reported by Archard Lias
26
This bug affects 5 people
Affects Status Importance Assigned to Milestone
Pyflakes
New
Undecided
Unassigned

Bug Description

Would it be possible to ignore variables named "_" from "Unused Variable" checks?

I'm not really sure whether "_" is officially specified anywhere. But I do see it a lot and in a lot of languages including python to denote a variable to be ignored (place holder dummy). PyLint implements this pseudo(?)-convention (? second-hand knowledge).

Would be great, since I would love to be explicit about "things returned but not used" instead of just calling without assignment which looks tantamount to "this function does never return anything / does not contain a return statement". A great plus in signature visibility throughout the code, specially for first-time readers.

Best regards,
~arlias

Tags: wishlist
Revision history for this message
Florent (florent.x) wrote :

A good practice is to provide a snippet with some details. A minimum is probably:
 * software version
 * actual behavor
 * expected behavior

Or I will just triage the issue as works-for-me, since I don't have any alert with this snippet:

    def get_extension(fname):
        (__, __, ext) = fname.partition('.')
        return ext

Revision history for this message
Florent (florent.x) wrote :

I would like to nuance the statement about "_" being a variable to be ignored.

In lot of places you also find "_" as the gettext shortcut for i18n of textual content.
So I changed my habits 2 years ago, and now I use two underscores for ignored variables.

And I only use it when I need to unpack a tuple and I don't want all the parts, like the example posted in my previous message.

Revision history for this message
Archard Lias (archardlias) wrote :

Right! had forgotten about internationalization. I don' really see both cases collide though. At least not visually. As a matter of reassignment it does though. Aliasing in that case is up to the user in that case though. Depending on local conventions.

============================================
# CASE 1
def myfunc():
    return 1, 2

a, _ = myfunc()

# CASE 2
def myfunc():
    return 'something'

_ = myfunc() # Func returns something I dont care
              # about in this particular case.

# CASE 3
# Catching the first elements with a, b, *remainder = (...) is fine. But in
# this case we have to define a placeholder to access the first and third
# element ignoring the second. Checker then tells us that that the second
# element is unused in code block to follow.
def myfunc():
    return (1, 2, 3)

a, _, b = myfunc()

# Could be also used for unpacking of tuples, but thats a case where it
# would be probably better to stick to slicing.

def myfunc():
    # mentioned usecase
    import internationalization as _
    print(_("internationalized string"))

    # naming alternative
    import internationalization as tl
    print(tl('internationalized string'))

    _ = myfunc()

def func_causing_problems():
    _ = myfunc()

    # ...
    return 123

==============================================================================
>>>
undertest.py:3:1: E302 expected 2 blank lines, found 1
undertest.py:9:1: E302 expected 2 blank lines, found 1
undertest.py:20:1: E302 expected 2 blank lines, found 1
undertest.py:42:5: F841 local variable '_' is assigned to but never used

1) global scope: OK
2) local scope: <F841>

flake8 version: 2.1.0-2
pyflakes version: 0.8.1-1
All from debian jessie/sid.

Revision history for this message
asmeurer (asmeurer) wrote :

pyflakes already doesn't warn about unused variables from unpacking. For a single value, why don't you just write

func()

instead of

_ = func()

Every function returns a value, but if you don't care about it, then don't assign it to anything. _ = func() is just confusing, and much less readable.

Revision history for this message
Archard Lias (archardlias) wrote :

True, in any case you get a "None". But what of being explicit about ignoring a non "None" return?

Is this case;
    log.debug("Postumous Call at Signal {0}".format(self._signal))
    ret = self.__call_signal(*args, **kwargs)
    _ = self.__call_slots(ret)
    return ret
less readable than:
    log.debug("Postumous Call at Signal {0}".format(self._signal))
    ret = self.__call_signal(*args, **kwargs)
    self.__call_slots(ret)
    return ret
?

I argue, it is less a question whether one or the other style is more correct or readable, than a matter of consistently ignoring assignments on "_", regardless of context. The individual use case is up to local conventions. Some might choose the classical, call without assignment way of formatting code, others might use the "ignorable placeholder" approach (as seen more in functional programming).

On a personal note, I do think that it "can" be more readable as well as "can be" more documenting "at times" to be explicit about ignoring a return value.

On a different note; testing unpacking ignore of
_, b = callable()
does not check the further usage of "b", ignoring everything in the assignment all together (not only "_").

Thankful for your patience,
best regards,
~arlias

Revision history for this message
Archard Lias (archardlias) wrote :

Ughh... try to picture the above code example with aligned "=".

Revision history for this message
asmeurer (asmeurer) wrote :

Assigning to _ for no reason is way less readable. It's not only confusing ("wait, does it somehow matter if this is assigned to a variable?"), but in my opinion the best way to not hide that a function just performs side-effects and doesn't return anything is to *not* assign it to something.

And aligned = are not PEP 8, so you're not going to get me on that argument.

Revision history for this message
Archard Lias (archardlias) wrote :

True, this is like "_" unpacking no PEP8. I'm just asking for the feasibility of "_" to be acknowledged as the relative unwritten convention for "ignored assignment". Whether it benefits someone in its local conventions or not is up to them. Now it has to obviously not intrude against PEP8 or other conventions that might conflict with it, which at the moment I see internationalization and the case of aliasing gettext(). Both of them do not conflict with the check itself though. They conflict if sharing the approach between different use cases in the same code base, which is again purely a issue of local conventions (outside of PEP8, so no deal breaker conflict there).

As for "=" alignment; never intended to pass it as PEP8, on the contrary let mi state it clearly; it is not... I just rigorously use it to ease vertical reading of the code. It does not pose part or relevance on the original matter. Had just problems between what I expected to be displayed and what actually got displayed in the comment.

Now to my stance at this point;
Even though the view on this unofficial convention is split, this is one of the two or more faces to it. It does not conflict but it requires your effort to support. If it is not worth it in your eyes or if you think it is actually a bad thing, it is fine by me. It is certainly difficult to set a convention objectively, since it is inherently subjective in nature. It is like agreeing on taste of design or beauty... again, difficult and requires the tolerance to walk away. Which I'm more than willing to do. No more nagging ;).
On that note, please close this feature request if you consider it to be a "not to become".

Thank you for your efforts and ongoing work on pyflakes (which has been and will continue to be an incredibly useful tool for me). Also thank you for already supporting "_" in unpacking and iterations/list/dict comprehensions. And also for your time in this discussion.

Best wishes,
~arlias

Revision history for this message
Kurtis Rader (krader) wrote :

It is a common idiom at my company to assign to the underscore var as a way to explicitly indicate a variable is unused. For example, you might have a method that overrides a base class method but which contains vars your implementation doesn't need:

    def MethodX(self, needed_var, unneeded_var):
        _ = unneeded_var
        ...

This makes it clear to the reader that the variable is being deliberately ignored and silences pylint. It would be nice if pep8 also recognized this convention. I'd like to propose a one line change that achieves this result:

diff --git a/pyflakes/checker.py b/pyflakes/checker.py
index 7055832..5bbc6b5 100644
--- a/pyflakes/checker.py
+++ b/pyflakes/checker.py
@@ -211,7 +211,7 @@ class FunctionScope(Scope):
     @ivar globals: Names declared 'global' in this function.
     """
     usesLocals = False
- alwaysUsed = set(['__tracebackhide__',
+ alwaysUsed = set(['_', '__tracebackhide__',
                       '__traceback_info__', '__traceback_supplement__'])

     def __init__(self):

Revision history for this message
Florent (florent.x) wrote :

I don't see the added value of "_ = unneeded_var" compared with a comment

  # Argument "unneeded_var" not used here

And if you use Flake8 you can easily configure it to ignore "_" if it matches your preferences.

Revision history for this message
Francesco Visin (fvisin) wrote :

Consider if you have 30K lines of company code that adopt that convention. You wouldn't want to replace all of the instances with a comment + the edited line, would you? It is a common convention in python, I don't see what's the harm of adding one line to fix this problem for those who have it.

Revision history for this message
Phil Frost (bitglue) wrote :

I don't think it's that common, but if we get a lot of comments to the contrary here, we can reevaluate.

Revision history for this message
Ian Cordasco (icordasc) wrote :

I have to agree with Florent and Phil. We've had 3 people complain over the course of ~16 months. That's certainly 3 more than any of us would like, but it's also 3 out of approximately 7.2 million downloads (which do not correspond to users) of pyflakes and almost 6 million downloads of Flake8 (which accounts for some percentage of pyflakes downloads which I won't attempt to estimate).

Revision history for this message
Archard Lias (archardlias) wrote :

Still, it is not a change that is in direct or indirect violation of PEP8 (its well within the unspecified realm).
Secondly, I cannot think of a use-case that conflicts with this change.
Third, there are people who use this, specially in partial unpacking that would benefit from it. It is even handled this way within list comprehensions. Why not stay consistent outside of them.

And finally the amount of people does not necessarily reflect the "necessity" of a change. I'd say that is a stance of denial, instead of a stance that is willing to evaluate the possibility to cater to a necessity without conflicting with anyone else. Is there a conflict between this and some other use-case or specification?

Revision history for this message
asmeurer (asmeurer) wrote :

One would have to concede that adding this probably wouldn't introduce any new false negatives (at least I've never used or seen _ as a variable except in unpacking and as the previous output in the interactive shell). It does seem odd, though, at least to my eyes. I'm sure PEP 8 doesn't even mention it because it's a relatively obscure thing. I can't speak for Guido, but I have a feeling that if it did say something about this it would recommend against it. I'm curious if this practice of assigning a function to an unused variable instead of just calling is common in other languages, or where in particular it comes from.

I also don't agree with Ian's argument. Three people taking the time to comment on an issue tracker request actually corresponds to a lot of real users, in my experience. You can go through can count the number of people who have commented on other, more important pyflakes issues, and you would see similar numbers.

With that being said, I'm still -0.5 to this (if my opinion counts for anything, which it probably doesn't, since I'm not actually a core pyflakes developer).

Revision history for this message
Phil Frost (bitglue) wrote :

> Is there a conflict between this and some other use-case or specification?

It's work to implement and maintain, and that's a reason to not do it.

Also, I can't rule out the possibility that people are assigning to _, and would want to know if that's unused. I would say that's an uncommon use case, but then so is this. And this isn't an entirely hypothetical guess: _ is a common common identifier for gettext-like functions.

> I'd say that is a stance of denial

It's not denial, it's laziness and skepticism. Please remember that this is an open source project made for your benefit entirely for free. Your success in getting this change implemented will depend more on how much work you put into it than how vehemently you argue for it.

You can do two things to help.

Firstly, research. What is the relative frequency of _ being used as an "intentionally unused" identifier (as you are), versus using _ as an ordinary identifier that just happens to be an underscore (gettext)? You'll need to provide real links to real code, preferably projects used by a lot of people and regarded as canonical sources. If you can demonstrate that a lot of people have your problem, and that addressing your problem won't create a lot of problems for other people, that will certainly make the case a lot stronger.

Secondly, you can submit a patch implementing the change you seek and with tests. That will significantly lower the barrier to implementing this change.

If you do both of those things, I guarantee this change will be in the next release.

Revision history for this message
Archard Lias (archardlias) wrote :

@asmeurer
First time I saw heavy use of it was in Haskell, which heavily relies on currying (python: functools.partial) to build functional inheritance (don't take my word on that last term, just trying to name a concept there). You basically use it to denote unused unpacks. One pivot point of the argument here is some of the nicer properties of functional programming have reached Python... hence the discussion.

@Phil
* PEP8 does not touch the topic of unused code at all. Flake does though, fortunately (F841).
* Unfortunately you're right on that. Assignment to _ is not to be ruled off. But you could argue that its an semantically information-less var name, hence highly probable of being discarded. As for gettext; since a callable it should not pose any false negatives.
* You're absolutely right. Then again the chance of a contribution being accepted around something that touches the outside of a clear rule set, depends a lot on consensus and can end up in a huge waste of time because of "personal opinion" and the requirement of research to overcome that, with no guarantees in the end. Quite frankly the balance of this issue in this last regard vs the benefit does not make it worth the effort for me. I guess that even in this regard the "invisible hand" holds true over the evolution of a open source project. Lazyness way to go ;).

Regardless of any of this, thanks a lot for this project and the time you pour into it, I'm very well aware of what it means effort-wise and value-wise and extremely grateful for it.

Revision history for this message
Simon Ruggier (simon80) wrote :

I don't have real links to real code, used by a lot of people, but both Pylint[1] and Eclipse[2] already treat underscore and underscore-prefixed identifiers on local variables as implying that the variable is knowingly unused.

In this case, we can also say confidently that addressing this problem won't create problems for other people: the risk is limited only to false negatives on the unused variable check, and then only for codebases that name local variables with an underscore prefix for some reason other than to ignore unused variable warnings, in spite of existing conventions on that.

It's still totally fair for you to ask for a patch instead of implementing it for free, but I think the likelihood of someone submitting a patch would be much higher if you were to confirm that such a patch (with tests) would be accepted.

[1] http://programmers.stackexchange.com/a/139662
[2] http://stackoverflow.com/q/11486148

Revision history for this message
Daniel (gr4viton) wrote :

I have a usecase
I am writing a class to be inherited by two separate classes
there a is a method (meth) which needs two arguments in (A) implementation of a sub-class, but only one in the (B) implementation, like this:
```
class root():
  @staticmethod
  def meth(a, b):
    pass
  def __init__(self, a, b):
    self.meth(a,b)

class A(root):
  def meth(a, b):
    use(a, b)

class B(root):
  def meth(a, b):
    _ = b
    use(a)
```

I want to have the method (meth) defined with both arguments as it is called in the root class method (__init__) the same for both subclasses.

However, in class B, I would like not to use the variable (b).

I am not allowed to write flake8 ignores. So I have a warning..

Just saying..

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.