Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False Positive: Constant name "logger" doesn't conform to UPPER_CASE naming style #2166

Closed
Stevoisiak opened this issue Jun 4, 2018 · 29 comments

Comments

@Stevoisiak
Copy link
Contributor

Stevoisiak commented Jun 4, 2018

Steps to reproduce

  1. Import logging in a module.
  2. Define a module-specific logger as logger.
  3. Analyze the file with pylint.
import logging

logger = logging.getLogger(__name__)
logger.setLevel(logging.DEBUG)
logger.debug('debug message')

Current behavior

Pylint interprets logger as a constant and warns that it should be capitalized.

C:  3, 0: Constant name "logger" doesn't conform to UPPER_CASE naming style (invalid-name)

Expected behavior

A logger object should not be interpreted as a constant, so there should not be a warning for logger.

pylint --version output

Using config file C:\Users\svascellar\.pylintrc
pylint 1.9.0,
astroid 1.6.3
Python 3.6.4 (v3.6.4:d48eceb, Dec 19 2017, 06:04:45) [MSC v.1900 32 bit (Intel)]
@PCManticore
Copy link
Contributor

This is not a false positive. Pylint expects all variables at the module level to be upper case. This behaviour can be configured by passing an updated const-rgx setting in the configuration file.

@jtmiclat
Copy link

Just want to add that another solution is to add logger to good-names setting in the configuration file.

@mcallaghan-bsm
Copy link

Why does pylint assume all module variables are CONSTANT? This assumption feels invalid.
Consider:

this_path = os.path.dirname(os.path.abspath(__file__))

This is dynamic per module, useful as a global member in the module, but definitely NOT a constant.

@mcallaghan-bsm
Copy link

Just want to add that another solution is to add logger to good-names setting in the configuration file.

Also this is not a great solution as it only solves that single variable name.

@mcallaghan-bsm
Copy link

(can we re-open this issue report for discussion+decision+action?)

@anjneeksharma
Copy link

Yes i am also getting same issue on this line
session = boto3.Session()

@kkm000
Copy link

kkm000 commented Dec 27, 2019

@anjneeksharma has a very good point, module-level globals are first-class constructs. Maybe a better heuristics to suspect a constant is a variable that (a) assigned once and (b) to a (more or less loosely defined) constant on the rhs. Why, I'll just leave this here: :)

find $(python -c $'import sys\nfor p in sys.path: print(p)') -name __init__.py | xargs grep -PHn '^\p{Ll}[\p{L}\d_]*\s*='

70 matches in a nearly stock installation, and that is not even looking inside /usr/lib/python37.zip [EDIT: There's no such a file, it's just there in sys.path in Debian 10's build]. There are probably much more, as this misses e.g. _ident = ... , but is already significant number.

Another fairly less than uncommon pattern seen in __init__.py files is

temp=...
. . .
temp=...
. . .
del temp

pylint gets angry at such "constants", too. And, FWIW, it's used in astroid itself.

I grok it that static code analysis is an extremely unsimple thing, and doubly so given that Python lacks a syntactic concept of constant. This comment by @mcallaghan-bsm in this very thread is a good illustration of the semantic borderline: this_path it's not a constant in the sense that it depends on the module's location within the filesystem (or whatever abstraction behind the __file__ lies), and at the same time it is a constant, because assigning it a different value at runtime makes no sense. But it still seems this heuristic could be improved.

@texadactyl
Copy link

texadactyl commented Dec 30, 2019

@PCManticore Given your Jun 4, 2018 comment, please change the diagnostic from

Constant name "Some-Name" doesn't conform to UPPER_CASE naming style

to

Module-level identifier "Some-Name" doesn't conform to UPPER_CASE naming style

In my opinion, the upper-case requirement should only apply to constants, wherever they are defined. However, I can live with this quirk if the diagnostic was at least telling the real reason for flagging the statement.

My example source code:

""" Why is xvar thought to be a constant but zvar is not?
Answer: pylint was flagging a module-level identifier, not a constant.
"""
YCONST = 1
xvar = 0 # line 5

def main():
    """
    Not empty
    """
    global xvar # line 11
    zvar = 0
    if YCONST > 0:
        xvar += 1
        zvar += 1
    else:
        xvar -= 1
        zvar -= 1
    print(xvar, YCONST)

main()

if YCONST > 0:
    xvar += 1
print(xvar)

pylint stdout:

************* Module essai
essai.py:5:0: C0103: Constant name "xvar" doesn't conform to UPPER_CASE naming style (invalid-name)
essai.py:11:4: W0603: Using the global statement (global-statement)
essai.py:11:4: C0103: Constant name "xvar" doesn't conform to UPPER_CASE naming style (invalid-name)

------------------------------------------------------------------
Your code has been rated at 8.00/10 (previous run: 8.00/10, +0.00)

@mcallaghan-bsm
Copy link

@PCManticore , per community notes we should re-open this issue for re-consideration.

@PCManticore
Copy link
Contributor

@mcallaghan-bsm @texadactyl Since 3422e4a we no longer emit invalid-name for module level non-constants. The rule is only constants such as numbers and strings. I'm not entirely sure what additional behaviour you'd want but in my opinion this is as good as it gets.

@mcallaghan-bsm
Copy link

@mcallaghan-bsm @texadactyl Since 3422e4a we no longer emit invalid-name for module level non-constants. The rule is only constants such as numbers and strings. I'm not entirely sure what additional behaviour you'd want but in my opinion this is as good as it gets.

nice, that precisely addresses my concerns (i guess the PR could also note it addresses this ticket or this is a duplicate of #3111 & #3132)

@texadactyl
Copy link

My point 8 days ago is that in my example, "xvar" was being called a constant (C0103). It isn't a constant. That's why I suggested the wording change in the diagnostic string.

@hernan-erasmo
Copy link

hernan-erasmo commented Jan 29, 2020

@mcallaghan-bsm @texadactyl Since 3422e4a we no longer emit invalid-name for module level non-constants. The rule is only constants such as numbers and strings. I'm not entirely sure what additional behaviour you'd want but in my opinion this is as good as it gets.

Maybe I'm missing something but is it correct that the following code

"""
Source: https://docs.python.org/3/library/argparse.html
"""

import argparse

parser = argparse.ArgumentParser(description='Process some integers.')
parser.add_argument('integers', metavar='N', type=int, nargs='+',
                    help='an integer for the accumulator')
parser.add_argument('--sum', dest='accumulate', action='store_const',
                    const=sum, default=max,
                    help='sum the integers (default: find the max)')

args = parser.parse_args()
print(args.accumulate(args.integers))

gives this output from pylint?

************* Module test_pylint
test_pylint.py:7:0: C0103: Constant name "parser" doesn't conform to UPPER_CASE naming style (invalid-name)
test_pylint.py:14:0: C0103: Constant name "args" doesn't conform to UPPER_CASE naming style (invalid-name)

------------------------------------------------------------------
Your code has been rated at 6.67/10 (previous run: 5.00/10, +1.67)

I'm running

C:\...>pylint --version
pylint 2.4.4
astroid 2.3.3
Python 3.8.1 (tags/v3.8.1:1b293b6, Dec 18 2019, 22:39:24) [MSC v.1916 32 bit (Intel)]

@HLFCode
Copy link

HLFCode commented Feb 7, 2020

Same problem on windows when I have multiple interfaces.
soco.discover uses:
socket.gethostbyname(socket.getfqdn())
which (in my case) returns
169.254.52.68
socket.gethostbyname_ex(socket.getfqdn())[2]
returns
['169.254.52.68', '192.168.56.1', '192.168.77.1', '192.168.15.1', '192.168.1.12']
the .68 entry is an Npcap loopback device (ie wrong), my ethernet interface is the last one.

I found a fix at stack overflow
This basically gets the interface ip address by

s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
s.connect(('8.8.8.8', 1))  # connect() for UDP doesn't send packets
local_ip_address = s.getsockname()[0]

So I changed my local copy of discovery.py as follows:

     _sockets = []
    # Use the specified interface, if any
    if interface_addr is not None:
        try:
            address = socket.inet_aton(interface_addr)
        except socket.error:
            raise ValueError("{0} is not a valid IP address string".format(
                interface_addr))
        _sockets.append(create_socket(interface_addr))
        _LOG.info("Sending discovery packets on %s", interface_addr)
    else:
        # Find the local network address using a couple of different methods.
        # Create a socket for each unique address found, and one for the
        # default multicast address
        addresses = set()
        try:
            skt = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
            #connect to a known address. 8.8.8.8 is a Google dns
            skt.connect(('8.8.8.8', 1))  # connect() for UDP doesn't send packets
            addresses.add(skt.getsockname()[0])
        except socket.error:
            pass
        try:
            addresses.add(socket.gethostbyname(socket.gethostname()))
        except socket.error:
            pass
        try:
            addresses.add(socket.gethostbyname(socket.getfqdn()))
        except socket.error:
            pass
        for address in addresses:
            try:
                _sockets.append(create_socket(address))
            except socket.error as e:
                _LOG.warning("Can't make a discovery socket for %s: %s: %s",
                             address, e.__class__.__name__, e)
        # Add a socket using the system default address
        _sockets.append(create_socket())
        # Used to be logged as:
        # list(s.getsockname()[0] for s in _sockets)
        # but getsockname fails on Windows with unconnected unbound sockets
        # https://bugs.python.org/issue1049
        _LOG.info("Sending discovery packets on %s", _sockets)

It now works a treat for me!
Tested on Windows and Raspbian...

@dickreuter
Copy link
Contributor

This still appear to be an issue.
log=logging.getLogger(name) still shows a false positive. Can this be reopened?

@texadactyl
Copy link

texadactyl commented Jun 26, 2020

@PCManticore
My point on 2020-01-07 is that Pylint is currently flagging any variable that is defined at the module level and is not spelled in UPPER_CASE, whether they are constants or variables.

A small program:

xvar = 0
xvar += 1
print(xvar)

Pylint flags variable xvar as follows:

C0103: Constant name "xvar" doesn't conform to UPPER_CASE naming style

Variable xvar is not a "Constant" because it changes value. The diagnostic is not true.

@plwalsh
Copy link

plwalsh commented Jul 25, 2020

The 3422e4a commit referenced above was first included in the v2.5.0 tag, as indicated on the commit's webpage. You need to upgrade your pylint to v2.5.0 or later for non-constant variables such as logger = logging.getLogger(__name__) to no longer be flagged.

@texadactyl
Copy link

@plwalsh .....

essai.py:

xvar = 0
xvar += 1
print(xvar)

pylint --version

pylint 2.5.3
astroid 2.4.2
Python 3.8.2 (default, Jul 16 2020, 14:00:26) 
[GCC 9.3.0]

pylint essai.py

************* Module essai
essai.py:1:0: C0114: Missing module docstring (missing-module-docstring)
essai.py:1:0: C0103: Constant name "xvar" doesn't conform to UPPER_CASE naming style (invalid-name)

@texadactyl
Copy link

texadactyl commented Jul 25, 2020

This issue should be re-opened unless I am missing something (possible!).
Alternative diagnostic:

essai.py:1:0: C0103: Module-level name "xvar" doesn't conform to UPPER_CASE naming style (invalid-name)

I could live with that even though I disagree with it.
My constants always "conform to UPPER_CASE naming style".

@plwalsh
Copy link

plwalsh commented Jul 25, 2020

@texadactyl: my response was directed at people like @dickreuter and @hernan-erasmo who were still having issues with non-string/non-number "module-level names". @PCManticore stated above that simple strings and numbers will still be treated as constants. Pylint is static code analysis; they can't execute the code to see if a variable truly holds a constant value or not, which is why he says this new rule is "as good as it gets." Additionally, rewording the diagnostic as you suggest would then invalidate the rule as they've now defined it. They're no longer saying that all "module-level names" should conform to UPPER_CASE, because they are excluding what they deem to be "module-level non-constants", such as expressions that are not simple strings or numbers. xvar = 0 is a simple number, and thus categorized as a "module-level constant", which is the best they can do statically. Maybe the hypothetical xvar being defined as a simple number at the module level while not being a constant should be re-evaluated?

@texadactyl
Copy link

@plwalsh
So, the bottom line is that "simple strings and numbers will still be treated as constants" even if they vary in value. I have no idea what is meant by 'simple' in that context. I will just put a filter in for C0103 since it is a totally useless diagnostic.

@plwalsh
Copy link

plwalsh commented Jul 26, 2020

@texadactyl: Truthfully, I think the real misnomer is the use of the word "constant" at all, since Python has no such concept. To me, it would make more sense for this rule to be talking about "globals" rather than "constants". Because I think what they're really saying with C0103 is that non-object globals (such as strings and numbers) should be UPPER_CASE. Whereas instantiated objects stored in globals (such as loggers and parsers) do not need to be upper case.

"simple" in the previous context was meant to refer to built-in data types like int and str, while "object" in this context is meant to refer to non-built-in types that are collections of data and methods.

@texadactyl
Copy link

Its okay, I black-listed C0103 - I'll not see it again. (-:

I more need help with issue #3750 because that is preventing some team members from using pylint. I am trying my best to get them to pylint to diagnose syntax and semantic errors before integration + execution. Yes, we are heavy astropy users.

@dmail00
Copy link

dmail00 commented Aug 28, 2022

Am I missing something here. It has gone from being able to supply a regex to match only using upper or lower case, to no C0103 reporting of pascal/camelcase naming at the global scope.

fooBar = ["something"]

def foobar():
    fooBar[0] = "something else"
    fooBar.append("what ever")
    
************* Module check_globals
check_globals.py:1:0: C0114: Missing module docstring (missing-module-docstring)
check_globals.py:3:0: C0116: Missing function or method docstring (missing-function-docstring)

------------------------------------------------------------------
Your code has been rated at 5.00/10 (previous run: 5.00/10, +0.00)

The following was run with 2.4.4 and 3.15.0

FooBar = ["something"]

def foobar():
    FooBar[0] = "something else"
    FooBar.append("what ever")
    somethingElse = ""

With 2.4.4 it was at least detecting that the name in the global scope did not conform, although reporting it as a Constant

pylint --variable-naming-style=snake_case check_globals
************* Module check_globals
check_globals.py:1:0: C0114: Missing module docstring (missing-module-docstring)
check_globals.py:1:0: C0103: Constant name "FooBar" doesn't conform to UPPER_CASE naming style (invalid-name)
check_globals.py:3:0: C0116: Missing function or method docstring (missing-function-docstring)
check_globals.py:6:4: C0103: Variable name "somethingElse" doesn't conform to snake_case naming style (invalid-name)
check_globals.py:6:4: W0612: Unused variable 'somethingElse' (unused-variable)

With 3.15.0 it doesn't care.

pylint --variable-naming-style=snake_case check_globals
************* Module check_globals
check_globals.py:1:0: C0114: Missing module docstring (missing-module-docstring)
check_globals.py:3:0: C0116: Missing function or method docstring (missing-function-docstring)
check_globals.py:6:4: C0103: Variable name "somethingElse" doesn't conform to snake_case naming style (invalid-name)
check_globals.py:6:4: W0612: Unused variable 'somethingElse' (unused-variable)

@DanielNoord
Copy link
Collaborator

@dmail00 This sounds like a regression. Is this change only on 2.15?

@mbyrnepr2
Copy link
Member

mbyrnepr2 commented Aug 29, 2022

I believe this is an intentional change introduced as far back as 2.5.0.
Please see #3111 for discussion 3422e4a for the actual commit.

@dmail00
Copy link

dmail00 commented Aug 29, 2022

@DanielNoord
As @mbyrnepr2 says this is due to a change introduced in 2.5.0 reguarding global names and constness, hence comparing with 2.4.4 the release prior to the change.

@mbyrnepr2 I understand the logic behind the change, but for it to not care about the name of something in the global scope is, in my opinion, an error. I specified snake_case on the command line for variables and whilst pylint may not be able to detect if it is a variable or constant it should report some error number as it does not match UPPER_CASE or snake_case.
Prior to 2.5.0 I could specify the regex for consts

"--const-rgx=(([a-z_][a-z0-9_]{2,30})|([A-Z_][A-Z0-9_]{2,30})|(__.*__))$"

Then I would patch the message in a custom reporter class and report that a global name doesn't conform to the naming standards. This is something which I use in an educational setting as part of a test stratergy for student's work. After seeing the change introduced in 2.5.0 I thought that I may no longer need to do this and could just rely on the standard reporting, but for it to not care about global names seems bizarre, it is a variable after all. The library went from reporting incorrectly module level names as constants to ignoring module level names which have the same conventions to functions as per PEP-8.[1][2]

In my situation, I can not use anything greater than 2.4.4 with this change.

[1] https://peps.python.org/pep-0008/#global-variable-names
[2] https://peps.python.org/pep-0008/#function-and-variable-names

@mbyrnepr2
Copy link
Member

Indeed @dmail00 I think the change which led to no warning being emitted for global non-constant assign-names was unintentional; we should fix that.

In your example, changing the list to a string will emit the warning as expected because it is seen as a constant. We need to extend that logic to also consider assigning non-constants.

@DanielNoord
Copy link
Collaborator

It would probably best to open a new issue for this as this will likely get lost otherwise.

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

No branches or pull requests