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

N806 Constant inside function false positive #87

Closed
fallenflint opened this issue Dec 9, 2018 · 3 comments
Closed

N806 Constant inside function false positive #87

fallenflint opened this issue Dec 9, 2018 · 3 comments

Comments

@fallenflint
Copy link

Accroding to PEP8, constants inside functions/method are not forbidden.

Following case fails test

class TestCase:
    def test_data:
        DATASET = {...}
        ... DATASET usage (may be multiple times)
@sigmavirus24
Copy link
Member

In the future, please link to what you're trying to reference. The two relevant sections, as far as I'm concerned are:

The former would indicate that DATASET here is a variable because (as the latter says) constants are (usually) defined at the module level. Analogously, DATASET would be a constant if defined at the class level.

Since DATASET is a variable, it should be all lower-case. Therefore, I don't believe this is a false-positive. You've provided no evidence of your point of view, so I'm closing this.

As I said on #86, we're not here to resolve intra-team fights. If your team doesn't like this, ignore N806.

@lordmauve
Copy link
Contributor

One reason you shouldn't spell locals like a constant is that you still pay a cost to construct the object (dict or set in your example above) each time the function is called.

In CPython, a couple of types (eg. tuples and list and set on the right hand side of in) are stored as constants in the code object. But that's an optimisation, and would be implementation dependent.

IMHO, the visual clue that the object is not being constructed at import time is valuable.

@fallenflint
Copy link
Author

@lordmauve, thanks for your opinion, but let me explain.
Since it's test case, performance doesn't matter, the choice is binary: either it's constant semantically (and I write it in upper-case) or it's a variable (and should be lower-cased).

Although import-time construction clue is quite noteworthy.

@sigmavirus24, Sorry, but I was pretty sure that including link to pep8 is redundant here, since this is a package, based on PEP8.
Sorry again, but it have absolutely nothing to do with team fights. It's all about fundamental PEP8 understanding. Moreover, other people could and would reference this tool trying to find out how to understand PEP8's point of view on method-level constants. Like me, cause my primary goal isn't to fight any debates, my goal is to find out the truth.

Next, as I said in #86, "usually" doesn't mean "always" and in no way forbids another manner. So it's obvious (at least for me), the primary criterion to consider, what is constant and what is not - is semantic and purpose. E.g. if value is not supposed to be changed - it's constant. And furhter, if PEP really prohibits constants elsewhere except, the constant simply should be moved to module-level.
But there's no evidence that PEP8 has anything against it. Moreover there are a lot of examples in solid mature code written in python, using constants otherwise than on module-level. For example, django models choices.

You here not to help someone to win debates and resolve intra-team fights, I've got the idea. But I'me here just to point that misunderstanding of word "usually" in pep8 and discuss if it's not misunderstanding. And to prevent anyone from wrong speculations.

And yes, I udnerstand that it's not trivial task to detect, what is constant and what is variable. Thanks for your time

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

3 participants