PyCQA / pep8-naming

Naming Convention checker for Python
pypi.python.org/pypi/pep8-naming
Other
500 stars 187 forks source link

metaclasses and functions returning classes #5

Open flying-sheep opened 11 years ago

flying-sheep commented 11 years ago

python is a dynamic language. functions may return classes, and instantiating metaclasses also yields classes.

since we can’t know what a function returns, VariablesInFunctionCheck should not yield an error if a variable name matches MIXEDCASE_REGEX, e.g.:

from sqlalchemy.orm import sessionmaker
Session = sessionmaker(bind=engine)
stephan-hof commented 11 years ago

So the snippet you posted happens within a function, right ? Something like this

def function():
    from sqlalchemy.orm import sessionmaker
    Session = sessionmaker(bind=engine)

Are you suggesting to disable the whole 'VariablesInFunctionCheck' ?

I mean PEP-8 is clear regarding instance variables, but not about variables in functions. I assumed the same rule and implemented this check.

flying-sheep commented 11 years ago

well, sessionmaker is mostly called toplevel, but there’s no reason why a class-returning function couldn’t be called in functions.

i suggest that functions have variable_names, classes have ClassNames, and:

  1. toplevel variables are allowed to have CONSTANT_NAMES and ClassNames.
  2. function level variables are allowed to have variable_names and ClassNames.

so the following code should be allowed, but arbitrary names of course not.

from . import MyMetaClass, DB_PATH
import sqlalchemy

ENGINE = sqlalchemy.create_engine(DB_PATH)
Session = sessionmaker(bind=ENGINE)

def commit_stuff():
    MyClass = MyMetaClass()
    my_object = MyClass()

    my_session = Session()
    my_session.add(my_object)
    my_session.commit()
sigmavirus24 commented 9 years ago

So we currently have a special case for namedtuple. I wouldn't be adverse to eliminating that special case and allowing top-level variables to have all caps or "ClassNames" as you described them. I'm pretty sure it won't be easy though.

pdc commented 1 year ago

I just tried adding pep8-names to a Django project and got a screenful of complaints about lines like this:

FooBar = apps.get_model("foobars", "FooBar")

Where get_model returns a class.

I can add #noqa: N806 to all of them, but it would be nice if I didn't have to. Some alternatives off the top of my head are

sigmavirus24 commented 1 year ago

I just tried adding pep8-names to a Django project and got a screenful of complaints about lines like this:

Tools don't complain, they tell you information you asked them to give you

FooBar = apps.get_model("foobars", "FooBar")

Where get_model returns a class.

I can add #noqa: N806 to all of them, but it would be nice if I didn't have to. Some alternatives off the top of my head are

  • (As suggested above) Allow variable assignments to use the capitalization of classes or constants and assume the programmer knows what they mean;

Except for the cases we've already caught and helped standardize for projects and the sudden lack of a computer telling someone they're violating the project/team's agreed upon convention.

  • Know whether the right-hand-side expression returns a type (which may require more analysis of the AST than is convenient);

No analysis of the AST will tell us this. Please don't spread misinformation.

  • Some other way of enumerating which functions and methods return types;

Yep, let's depend on a language server or mypy to give us that information, otherwise let's rebuild the core of those projects to do it.

  • Allow FooBar: type = … to make it explicit FooBar is a type ;

This might be workable but is as useful as a noqa - both of which are silencing a tool.

  • Some comment that says FooBar in the next line is a type (but that is not much better than noqa).
pdc commented 1 year ago

Thanks for your feedback. Just to be clear, #noqa: N806 is fine