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

Bug #1162031 reported by Bryan Silverthorn
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Pyflakes
New
Wishlist
Unassigned

Bug Description

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")

Revision history for this message
Florent (florent.x) wrote :

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 issue #1153355.
But it is not a bug.

Changed in pyflakes:
status: New → Incomplete
Revision history for this message
Bryan Silverthorn (bcsilverthorn) wrote :

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).

Revision history for this message
Florent (florent.x) wrote :

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.

Revision history for this message
Bryan Silverthorn (bcsilverthorn) wrote :

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.)

Revision history for this message
Florent (florent.x) 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
Revision history for this message
Bryan Silverthorn (bcsilverthorn) wrote : Re: [Bug 1162031] Re: "import as" can cause spurious "imported but unused" warnings

Sounds fair. Appreciated the discussion.

On Sat, Mar 30, 2013 at 9:13 AM, Florent <email address hidden> 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
>

Florent (florent.x)
Changed in pyflakes:
importance: Undecided → Wishlist
Revision history for this message
John Vandenberg (jayvdb) wrote :

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 comment #5 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")

Revision history for this message
Bryan Silverthorn (bcsilverthorn) wrote :

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!

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.