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

"camelcase imported as lowercase" failure in an import of a package or module #201

Open
ericbn opened this issue May 27, 2022 · 12 comments
Open

Comments

@ericbn
Copy link

ericbn commented May 27, 2022

For example, this code:

import SalesforcePy.commons as sfc

fails with:

./test.py:1:2: N813 camelcase 'SalesforcePy.commons' imported as lowercase 'sfc'

Shouldn't pep-naming just look at the package name being imported (commons) instead of looking at the parent package names too consider this is an import of a package or module, and then not fail in this case?

I'm assuming this rule exists for when a class name is being imported, as discussed in https://stackoverflow.com/a/37590149/2654518. In this case:

from SalesforcePy.commons import BaseRequest as baserequest

sure should fail with:

./test.py:1:2: N813 camelcase 'BaseRequest' imported as lowercase 'baserequest'
@ericbn

This comment was marked as outdated.

@ericbn

This comment was marked as outdated.

@sigmavirus24
Copy link
Member

Holding a sleeping newborn so I'm going to be terse and respond to exactly I've point you made

Shouldn't pep-naming just look at the package name being imported (commons) instead of looking at the parent package names too, and then not fail in this case?

In the example you gave you otherwise would have had a camelcase name to reference everywhere SalesforcePy.commons. You aliases it as a lowercase. I don't think that is a valid example of what you're trying to prove

@ericbn
Copy link
Author

ericbn commented Jun 1, 2022

Congratulations on your newborn! Don't mind paying attention to this issue at least for the next 3 years or so! :- )


Let me try to rephrase the issue. In from mod import NICE as GOOD and import mod.NICE as GOOD my understanding is that "mod.NICE is being bound as GOOD" in both cases. As least this is my interpretation from the import statement documentation, when it exemplifies:

import foo.bar.baz as fbb  # foo, foo.bar, and foo.bar.baz imported, foo.bar.baz bound as fbb
from foo.bar import baz    # foo, foo.bar, and foo.bar.baz imported, foo.bar.baz bound as baz

The issue is that from mod import NICE as GOOD does not yield any linting error, but import mod.NICE as GOOD does. Same for any other variations where "nice" and "good" have the same type of case. I'm thinking both constructs are equivalent, so linting should yield the same result for both (no error).

@sigmavirus24
Copy link
Member

So i think the disconnect between you and me is based on what an import might be used like if not allowed to a different casing.

Two examples you have have a fully qualified band that is now mixed case without any alias. Using the Salesforce library to start with, I believe that changing that to from SalesforcePy import common as sfc wouldn't trigger the error because without the alias you'd refer to that as common otherwise. Do you see my point?

I do think the import lower. GOOD as good is trickier given convention that no one would import a constant like that.

Again, she caveats if holding a newborn and replying on mobile with one have

@ericbn
Copy link
Author

ericbn commented Jun 1, 2022

I see what you mean. from mod import NICE allows referring to NICE directly, while import mod.NICE makes you refer to mod.NICE.

from SalesforcePy import common as sfc works, but I prefer import SalesforcePy.common as sfc because SalesforcePy.common are modules. I'm just aliasing those, like suggested here to import very.deep.module as mod.

Maybe we can simplify the question to: When you look at mod.NICE do you see an upper case name coming from a lower case name, or a camel case name? Likewise, what about import mod.NICE? PEP8 does not say anything about this specifically, and at the end this is just about dealing with non-PEP8-standard module names, like SalesforcePy in the wild...

@ericbn
Copy link
Author

ericbn commented Jun 2, 2022

I think I have a solution that makes more sense, in PEP8 terms, than the discussion I started above (and sorry if that was confusing).

Given that

when using from package import item, the item can be either a submodule (or subpackage) of the package, or some other name defined in the package, like a function, class or variable.

and

when using syntax like import item.subitem.subsubitem, each item except for the last must be a package; the last item can be a module or a package but can’t be a class or function or variable defined in the previous item.

(from https://docs.python.org/3/tutorial/modules.html#packages)

Then I propose:

  • When using from package import item as name, enforce that item and name have the same case type. This can be kept in rules N81{1,2,3,4,7}. They will just need to be more specific.
  • When using import item.subitem.subsubitem as name or import item as name, enforce that name is lowercase, because the last item can only be a module or a package. This will be a new rule (N819?).

@jparise
Copy link
Member

jparise commented Jun 7, 2022

Then I propose:

  • When using from package import item as name, enforce that item and name have the same case type. This can be kept in rules N81{1,2,3,4,7}. They will just need to be more specific.

  • When using import item.subitem.subsubitem as name or import item as name, enforce that name is lowercase, because the last item can only be a module or a package. This will be a new rule (N819?).

I think those generally sound like good rules, they sort of fall outside the bounds of PEP 8. The Package and Module Names section tells us how a (source) module should be named, but I don't know if we should be adding enforcement to import'ing side.

Your second proposed rule feels closest in spirit to PEP 8. I'm less sure about the first one.

@ericbn
Copy link
Author

ericbn commented Jun 7, 2022

The Package and Module Names section tells us how a (source) module should be named, but I don't know if we should be adding enforcement to import'ing side.

Agree. That's the point of the second proposed rule (not to force the case type to the importing side, only on the alias name side, since there are non-lowercase package names out in the wild, e.g. SalesforcePy)

If we also want to enforce the packages and modules in the code being linted to be lowercase, maybe we could check the directory and file names. But that might be a separate discussion...

Your second proposed rule feels closest in spirit to PEP 8. I'm less sure about the first one.

What are you not sure about the first proposed rule? The way I see it, rules N81{1,2,3,4,7} are already enforcing that aliases have the same case type as the imported item. I'm proposing these rules to be more specific (only be applied in imports in the form from package import item as name), and then to have a separate rule to specifically cover the module/package aliasing (the second proposed rule, to only be applied in imports in the form import item.subitem.subsubitem as name or import item as name).

@jparise
Copy link
Member

jparise commented Jun 7, 2022

Your second proposed rule feels closest in spirit to PEP 8. I'm less sure about the first one.

What are you not sure about the first proposed rule? The way I see it, rules N81{1,2,3,4,7} are already enforcing that aliases have the same case type as the imported item. I'm proposing these rules to be more specific (only be applied in imports in the form from package import item as name), and then to have a separate rule to specifically cover the module/package aliasing (the second proposed rule, to only be applied in imports in the form import item.subitem.subsubitem as name or import item as name).

I understand better now. I thought you were proposing that we change the check to enforce that the alias simply matches the case of the imported name, skipping the other naming rules.

@ericbn
Copy link
Author

ericbn commented Jun 8, 2022

Cool. Let me work on the code changes and propose then in a PR.

@ericbn
Copy link
Author

ericbn commented Feb 28, 2023

Hi. Can we have a second look at this? Maybe I didn't express myself well before.

The goal is to check the PEP 8 package and module names convention is applied to aliases of imported packages or modules.

So given import item as alias, the rule will make sure that alias is always lowercase -- regardless if the case of item --, considering alias is a package or module (alias) name and should follow:

Modules should have short, all-lowercase names. Underscores can be used in the module name if it improves readability. Python packages should also have short, all-lowercase names, although the use of underscores is discouraged.

We won't check for the case of item because developers won't have control of that name in case of third-party packages, but we will check the case of alias because that was a name given by the developer of the current code.

EDIT: This rule will be specific for the import item as alias syntax. Given that:

when using syntax like import item.subitem.subsubitem, each item except for the last must be a package; the last item can be a module or a package but can’t be a class or function or variable defined in the previous item.

(from https://docs.python.org/3/tutorial/modules.html#packages)

@ericbn ericbn changed the title "camelcase imported as lowercase" fails even when only parent package is camelcase "camelcase imported as lowercase" failure in an import of a package or module Mar 1, 2023
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