N812 for uppercase shorthand

[peterdemin]

I believe it's common practice to shorten long imports from selenium like it is done in Selenium docs: http://selenium-python.readthedocs.io/waits.html

But pep8-naming detects error:

from selenium.webdriver.support import expected_conditions as EC
^
1     N812 lowercase 'EC' imported as non lowercase
1

I think the intention of N812 was to avoid importing lowercase as CamelCase, but this one is actually UPPERCASE, so I think it's false positive.

[sigmavirus24]

I've been working on Python for several years and, anecdotally speaking, can't remember seeing this ever, let alone it being common practice. If there's more evidence of this outside of selenium's docs/users, I'm happy to consider this change. Otherwise, I'm not sure the effort would be entirely justifiable.

[peterdemin]

To be fair, I don't think it's a good practice. But its origin is not Selenium docs.
from BeautifulSoup import BeautifulSoup as BS
I'm sure I've seen the same pattern in ML field, something in scikit-learn, but I don't remember what exactly.
Anyway, N812 does not distinguish between CamelCase (intent) and short UPPERCASE (side-effect), so I don't quite understand what kind of justification you are looking for.

[sigmavirus24]

@peterdemin I'm sorry, I didn't do a great job explaining what I was asking for. In general, I'm happier to have false negatives than false positives but I'm happiest to have neither. For this specific error code, the guidance isn't exactly clear in the PEP. This is a grey area. For items like this, I would like to try to standardize on what the larger python community is already doing, as that would mean we're causing less pain. Selenium and BeautifulSoup both have wonderfully large user-bases. I'm sure to some extent they overlap as well. I'd love to see if this idiom is common outside of these communities and in the larger python community, and one way to do that is to leave this issue open for a while and continue to collect feedback from other folks. You don't have to do other work in tracking this information down.

My inkling (again, it's just a feeling) is that people may be more surprised that the examples you gave don't raise the error rather than that they do raise the error. But this is why I'd like to see who else sees this issue and comments on it. I'm frankly happy to add this if it proves out that this is something common that I've failed to see in my years.

---

As a note to other folks who may stumble across this thread:

• Reactions to the issue don't generate notifications which means if 30 people thumbs-up this, I may not see that until someone comments on the issue again.

• Comments that fall along the themes of "+1" or ":+1:" or "Why isn't this fixed yet?" aren't going to be helpful. Instead, it would be helpful to give examples of your code doing this that's causing problems and the impact this error is having on your project.

[jackrosenthal]

Having stumbled upon this thread, think I can offer my 2c:

I would be surprised if this tool did not detect this as an error. While it's most certainly a grey area in PEP 8, it results in having something that would normally be cased in lower as an uppercase constant... not something I would typically expect.

For projects which rely on that naming convention (e.g., if using example from Selenium docs), why not these projects just add to ignore list?

On a related side note, this should be OK::

from BeautifulSoup import BeautifulSoup as BS

BS could be CapWords or UPPERCASE as @peterdemin points out.

[joshkel]

As another data point, the standard Python docs suggest importing ElementTree as ET.

(I checked the BeautifulSoup docs, and importing that as BS isn't in the docs that I checked, but it appears to be quite common in practice.)

It seems like a good compromise to me to support importing CamelCaseNames as strict abbreviations (e.g., importing CamelCaseNames as CCN would be allowed) - that supports common use cases like ElementTree and BeautifulSoup without straying too far from the PEP8 recommendations.

But it's also easy to stick a # noqa:N814 at the end of imports like that, if you'd prefer to keep the pep8-naming behavior as is.

[5j9]

(In response to @sigmavirus24 at #99 (comment))

---

My lines of thinking:

We currently allow uppercase class names (related test), in that context, importing a ClassName as an abbreviated UPPERCASE form also sounds somewhat reasonable. (0.25+)

I have personally both seen and used imports like ElementTree as ET or BeautifulSoup as BS in small scripts, but I don't like to use them in production, they make the code harder to understand, IMO. (-0.5)

According to PEP8 CapWords is used for:

Class names should normally use the CapWords convention.
exception names and builtin constants
Names of type variables introduced in PEP 484 should normally use CapWords

I expect pep8-naming to also normally not allow importing CapWords as uppercase. (-0.25)
It should be possible for the user to explicitly ignore them, though.

When using acronyms in CapWords, capitalize all the letters of the acronym. Thus HTTPServerError is better than HttpServerError.

(+0.25?)

---

TLDR;

Overall, I'm not very sure and could go either way, but my current preference is for having a separate error code for CapWords imported as CW cases so that users who prefer to silence them can do so easily; and I think the related check should be active by default.

[sigmavirus24]

Overall, I'm not very sure and could go either way, but my current preference is for having a separate error code for CapWords imported as CW cases so that users who prefer to silence them can do so easily; and I think the related check should be active by default.

I like that a lot and am +1 on that actually.

[jparise]

Overall, I'm not very sure and could go either way, but my current preference is for having a separate error code for CapWords imported as CW cases so that users who prefer to silence them can do so easily; and I think the related check should be active by default.

I like that a lot and am +1 on that actually.

I took a crack at implementing this approach in #133.

[peterdemin]

Thanks, @jparise!

[saravanabalagi]

What about import torch.nn.functional as F?
Related: #226