pep8-naming
pep8-naming copied to clipboard
"camelcase imported as lowercase" failure in an import of a package or module
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'
Here are some test cases I believe exemplify the possible scenarios, based on the existing test cases:
#: Okay
import mod.good as nice
#: Okay
import Mod.good as nice
#: Okay
import mod.NICE as GOOD
#: Okay
import mod.Camel as Memel
#: N811:1:1
import mod.GOOD as bad
#: N812:1:1
import mod.good as Bad
#: N812:1:1
import Mod.good as Bad
#: N813:1:1
import mod.CamelCase as noncamel
#: N814:1:1
import mod.CamelCase as CONSTANT
#: N817:1:1
import mod.CamelCase as CC
Fix should be as simple as
diff --git a/src/pep8ext_naming.py b/src/pep8ext_naming.py
index 2755aaa..b2ad238 100644
--- a/src/pep8ext_naming.py
+++ b/src/pep8ext_naming.py
@@ -396,7 +396,7 @@ class ImportAsCheck(BaseASTCheck):
asname = name.asname
if not asname:
continue
- original_name = name.name
+ original_name = name.name.rsplit('.', 1)[-1]
err_kwargs = {'name': original_name, 'asname': asname}
if original_name.isupper():
if not asname.isupper():
Or
diff --git a/src/pep8ext_naming.py b/src/pep8ext_naming.py
index 2755aaa..b33eb78 100644
--- a/src/pep8ext_naming.py
+++ b/src/pep8ext_naming.py
@@ -397,6 +397,7 @@ class ImportAsCheck(BaseASTCheck):
if not asname:
continue
original_name = name.name
+ original_name = original_name[original_name.rfind('.') + 1:]
err_kwargs = {'name': original_name, 'asname': asname}
if original_name.isupper():
if not asname.isupper():
for something maybe faster and less clean.
I'm more than happy to create a PR once the idea is approved.
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
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).
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
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...
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 thatitem
andname
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
orimport item as name
, enforce thatname
is lowercase, because the last item can only be a module or a package. This will be a new rule (N819?).
Then I propose:
When using
from package import item as name
, enforce thatitem
andname
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
orimport item as name
, enforce thatname
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.
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 import
ing 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
).
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 formimport item.subitem.subsubitem as name
orimport 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.
Cool. Let me work on the code changes and propose then in a PR.
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)