2

Here is some code:

foo = "Bears"
"Lions, Tigers and %(foo)s" % locals()

My PEP8 linter (SublimeLinter) complains about this, because foo is "unreferenced". My question is whether PEP8 should count this type of string interpolation as "referenced", or if there is a good reason to consider this "bad style".

A. Wilson
  • 8,534
  • 1
  • 26
  • 39

3 Answers3

5

Well, it isn't referenced. The part that's questionable style is using locals() to access variables instead of just accessing them by name. See this previous question for why that's a dubious idea. It's not a terrible thing, but it's not good style for a program that you want to maintain in the long term.

Edit: It's true that when you use a literal format string, it seems more explicit. But part of the point of the previous post is that in a larger program, you will probably wind up not using a literal format string. If it's a small program and you don't care, go ahead and use it. But warning about things that are likely to cause maintainability problems later is also part of what style guides and linters are for.

Also, locals isn't a canonical representation of names that are explicitly referenced in the literal. It's a canonical representation of all names in the local namespace. You can still do it if you like, but it's basically a loose/sloppy alternative to explicitly using the names you're using, which is again, exactly the sort of thing linters are supposed to warn you about.

Community
  • 1
  • 1
BrenBarn
  • 242,874
  • 37
  • 412
  • 384
  • In this case, though, the string interpolation is explicit in the literal. I see where he is coming from in the other answer, when passing format strings that could be coming from anywhere, but that's still a different monster from having a string literal with explicit format tags and interpolation, even if they use the same mechanism. I can see the spiritual desire for having one way to evaluate this, but (here) locals() is a canonical representation of names that are explicitly referenced in a literal, so I feel like the argument can still go the other way. Am I just crazy? – A. Wilson Dec 11 '12 at 20:01
  • 1
    @A.Wilson: See my edited answer. Part of the point of that post is that if it's a big program, you probably will wind up not using literal format strings forever. If you know that won't be a problem, go ahead and do it, but the linter is there to warn you of things that might be a problem later. – BrenBarn Dec 11 '12 at 20:16
  • @BrenBarn: That's a great meta-point. Is the purpose of a linter to verify that your code technical complies with the literal text of PEP8, or that it avoids the problems PEP8 is trying to prevent? – abarnert Dec 11 '12 at 20:23
  • PEP8 is a Style Guide for Python Code, emphasis on _Guide_. The second section in it, titled [_A Foolish Consistency is the Hobgoblin of Little Minds_](http://www.python.org/dev/peps/pep-0008/#a-foolish-consistency-is-the-hobgoblin-of-little-minds) says "know when to be inconsistent" which in this context could mean simply ignore SublimeLinter's complaint. I can think of other cases where it would erroneously indicate such "problems". – martineau Dec 11 '12 at 20:37
  • Moving my acceptance down here after the edit, since it includes everything, but like mentioned with @abarnet's answer, I'm sad I can't indicate that they were both important. – A. Wilson Dec 11 '12 at 20:50
1

Even if you reject BrenBarn's argument that foo isn't referenced, if you accept the argument that passing locals() in string formatting should be flagged, it may not be worth writing to code to consider foo referenced.

First, in every case where that extra code would help, the construct is not acceptable anyway, and the user is going to have to ignore a lint warning anyway. Yes, there is some harm in giving the user two lint warnings to ignore when there's only actually one problem, especially if one of the warnings is somewhat misleading. But is it enough harm to justify writing very complicated code and introduce new bugs into the linter?

You also have to consider that for this to actually work, the linter has to recognize not just % formatting, but also {} formatting, and every other kind of string formatting, HTML templating, etc. that the user could be using. In practice, this means handling various very common forms, and providing some kind of hook for the user to describe anything else.

And, on top of that, even if you don't think it should work with arbitrarily-generated format strings, it surely has to at least work with l10n. How is that going to work? If the format string is generated by something like gettext, the linter has no way of knowing whether foo is referenced, unless it can check all of the translations and see that at least one of them references foo—which means it has to understand (or have hooks to be taught) every string translation mechanism, and have access to the translation database.

So, I would suggest that, even if you consider the warning spurious in this case, you leave it there anyway. At most, add something which qualifies the warning:

foo is possibly unreferenced, but in a function that uses locals()

abarnert
  • 354,177
  • 51
  • 601
  • 671
  • You've convinced me. I wish I could accept yours with BrenBarn's, though – A. Wilson Dec 11 '12 at 20:14
  • My answer pretty much explicitly depends on BrenBarn's; what would be _really_ nice is if there were a way to indicate that to SO, so accepting mine would automatically give him some kind of credit, move his answer up to second on the list, etc. – abarnert Dec 11 '12 at 20:16
  • 1
    @A.Wilson: A further thought on this: I think you want the qualified warning in _any_ case with `locals()`. Imagine a function `def foo(func): bar=3; func(locals())`. Is `bar` referenced? Even if the linter could somehow figure it out, a human reader cannot, and that's exactly what you want to flag. (But you probably do want to find a better wording for the warning than mine…) – abarnert Dec 11 '12 at 20:21
0

The following wouldn't make SublimeLinter happy either, It looks up each variable name referenced in the string and substitutes the corresponding value from the namespace mapping, which defaults to the caller's locals. As such it show the inherent limitation a utility like SublimeLinter has when trying to determine if something has been referenced in Python). My advice is just ignore SublimeLinter or add code to fake it out, like foo = foo. I've had to do something like the latter to get rid of C compiler warnings about things which were both legal and intended.

import re
import sys
SUB_RE = re.compile(r"%\((.*?)\)s")

def local_vars_subst(s, namespace=None):
    if namespace is None:
        namespace = sys._getframe(1).f_locals

    def repl(matchobj):
        var = matchobj.group(1).strip()
        try:
            retval = namespace[var]
        except KeyError:
            retval = "<undefined>"
        return retval

    return SUB_RE.sub(repl, s)

foo = "Bears"
print local_vars_subst("Lions, Tigers and %(foo)s")
martineau
  • 119,623
  • 25
  • 170
  • 301