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

"import as" can cause spurious "imported but unused" warnings #159

Closed
pyflakes-bot opened this issue Mar 29, 2013 · 9 comments
Closed

"import as" can cause spurious "imported but unused" warnings #159

pyflakes-bot opened this issue Mar 29, 2013 · 9 comments

Comments

@pyflakes-bot
Copy link

Original report by bcs26 on Launchpad:


Pyflakes will generate a spurious "imported but unused" warning for the following code:

import selenium as se
import selenium.webdriver

se.webdriver.Firefox("foo")

Note that Pyflakes will, correctly, not generate such a warning for the following:

import selenium
import selenium.webdriver

se = selenium

se.webdriver.Firefox("foo")

@pyflakes-bot
Copy link
Author

pyflakes-bot commented Mar 29, 2013

Original comment by florent.x (@florentx?) on Launchpad:


The "imported but unused" is not triggered on "import ... as ...".
Actually, it is triggered by the second import line, not the first one.

I agree this is slightly confusing because of #157.
But it is not a bug.

@pyflakes-bot
Copy link
Author

Original comment by bcs26 on Launchpad:


Agreed that the warning targets the second "import". The warning itself, however, does seem to be caused by the presence of "import as". The following (effectively identical) code, for example, correctly does not trigger a warning:

import selenium
import selenium.webdriver

selenium.webdriver.Firefox("foo")

Not sure I understand the comment about it not being a bug: an "unused" warning is generated, but both import lines are in fact essential (the code will raise an error if either is removed).

@pyflakes-bot
Copy link
Author

Original comment by florent.x (@florentx?) on Launchpad:


In the "import as" example, the name "selenium" is assigned in the global namespace but it is not used in the module. Only "se" is effectively used.
In the other example, the "selenium" name is assigned and is effectively used.

There are different ways to work around this and avoid putting an unused name in the globals. For example:

  from selenium import webdriver
  webdriver.Firefox("foo")

Or another example:

  import selenium as se
  __import__('selenium.webdriver')
  se.webdriver.Firefox("foo")

Or you keep the code as-is, and you just ignore this pyflakes message.

@pyflakes-bot
Copy link
Author

Original comment by bcs26 on Launchpad:


Ok, it honestly sounds like we just disagree on this.

If you'd like, I'm happy to call this a feature request: it would be helpful if pyflakes did not generate a warning for this code, since the only alternative is adding code solely to suppress the warning.

That said, while the name may be unused, the import operation is obviously essential. Using __import__("...") to avoid introducing the name seems a bit silly, especially because pyflakes does not in general warn about unused top-level names---so, in this case, it is going out of its way to warn about reasonable code.

(As a perhaps-interesting aside, using "del selenium" to remove the name generates a different warning.)

@pyflakes-bot
Copy link
Author

Original comment by florent.x (@florentx?) on Launchpad:


I keep the issue open, since you don't like the alternative examples which were proposed:

import selenium
import selenium.webdriver
se = selenium
se.webdriver.Firefox("foo")

(or)

from selenium import webdriver
webdriver.Firefox("foo")

For the other remark, regarding the unassignment of globally imported objects, I opened bug 1162161 and attached a patch to it.

@pyflakes-bot
Copy link
Author

Original comment by bcs26 on Launchpad:


Sounds fair. Appreciated the discussion.

On Sat, Mar 30, 2013 at 9:13 AM, Florent wrote:

I keep the issue open, since you don't like the alternative examples
which were proposed:

import selenium
import selenium.webdriver
se = selenium
se.webdriver.Firefox("foo")

(or)

from selenium import webdriver
webdriver.Firefox("foo")

For the other remark, regarding the unassignment of globally imported
objects, I opened bug 1162161 and attached a patch to it.

** Changed in: pyflakes
Status: Incomplete => New

--
You received this bug notification because you are subscribed to the bug
report.
https://bugs.launchpad.net/bugs/1162031

Title:
"import as" can cause spurious "imported but unused" warnings

To manage notifications about this bug go to:
https://bugs.launchpad.net/pyflakes/+bug/1162031/+subscriptions

@pyflakes-bot
Copy link
Author

pyflakes-bot commented Nov 25, 2015

Original comment by jayvdb (@jayvdb?) on Launchpad:


It is possible, but not easy, for pyflakes to detect that import selenium.webdriver is altering se. But even if pyflakes detected this, IMO it should still emit an error for it. Instead of "imported but unused", it should report "imported submodule a.b only used via c.b."

IMO the two options provided in #159 (comment) are the correct "flake free" code, and IMO this is the most desirable:

from selenium import webdriver
webdriver.Firefox("foo")

If a more descriptive prefix is desirable, that could be:

from selenium import webdriver as selenium_webdriver
selenium_webdriver.Firefox("foo")

@pyflakes-bot
Copy link
Author

Original comment by bcs26 on Launchpad:


Both of those alternatives are totally reasonable... but at least IMO they don't feel super great.

For many larger packages (e.g. numpy) an "import numpy as np" style is extremely common, but you still need to import various submodules. If you apply the pattern above, you end up with "import numpy.modulea as np_modulea", "import numpy.random.moduleb as np_random_moduleb", etc. That obviously works but ends up creating two independent styles of usage. Not to mention the repetitive boilerplate in the actual imports.

Ultimately it feels like working around an implementation limitation on the linter side. Fine if that's what it takes, better if it's unnecessary!

@asottile
Copy link
Member

the second import is purely for side-effects -- I believe pyflakes is correct to flag this as a programming error/smell

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

2 participants