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

metaclasses and functions returning classes #5

Open
flying-sheep opened this issue May 31, 2013 · 6 comments
Open

metaclasses and functions returning classes #5

flying-sheep opened this issue May 31, 2013 · 6 comments

Comments

@flying-sheep
Copy link

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Member

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
Copy link

pdc commented Nov 29, 2022

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

  • (As suggested above) Allow variable assignments to use the capitalization of classes or constants and assume the programmer knows what they mean;
  • Know whether the right-hand-side expression returns a type (which may require more analysis of the AST than is convenient);
  • Some other way of enumerating which functions and methods return types;
  • Allow FooBar: type = … to make it explicit FooBar is a type ;
  • Some comment that says FooBar in the next line is a type (but that is not much better than noqa).

@sigmavirus24
Copy link
Member

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
Copy link

pdc commented Nov 30, 2022

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

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

No branches or pull requests

4 participants