Opened 20 months ago

Closed 19 months ago

Last modified 19 months ago

#21302 closed Cleanup/optimization (fixed)

Fix or ignore remaining "import *" statements

Reported by: timo Owned by: timo
Component: Core (Other) Version: master
Severity: Normal Keywords:
Cc: bmispelon@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The remaining import * statements are primarily to put all the objects from one module into another namespace for convenience imports. I'm unsure if we're fine with using import * to accomplish this or if we should be explicit. We should either replace import * with the appropriate imports, or mark each line as # NOQA so we can remove F403 from the flake8 ignore list in setup.cfg:

./django/forms/__init__.py:6:1: F403 'from django.forms.fields import *' used; unable to detect undefined names
./django/forms/__init__.py:7:1: F403 'from django.forms.forms import *' used; unable to detect undefined names
./django/forms/__init__.py:8:1: F403 'from django.forms.models import *' used; unable to detect undefined names
./django/forms/__init__.py:9:1: F403 'from django.forms.widgets import *' used; unable to detect undefined names
./django/forms/extras/__init__.py:1:1: F403 'from django.forms.extras.widgets import *' used; unable to detect undefined names
./django/db/migrations/__init__.py:2:1: F403 'from operations import *' used; unable to detect undefined names
./django/db/models/__init__.py:9:1: F403 'from django.db.models.aggregates import *' used; unable to detect undefined names
./django/db/models/__init__.py:10:1: F403 'from django.db.models.fields import *' used; unable to detect undefined names
./django/db/models/sql/__init__.py:2:1: F403 'from django.db.models.sql.subqueries import *' used; unable to detect undefined names
./django/db/models/sql/__init__.py:3:1: F403 'from django.db.models.sql.query import *' used; unable to detect undefined names
./django/contrib/gis/forms/__init__.py:1:1: F403 'from django.forms import *' used; unable to detect undefined names
./django/contrib/gis/geos/prototypes/__init__.py:21:1: F403 'from django.contrib.gis.geos.prototypes.misc import *' used; unable to detect undefined names
./django/contrib/gis/geos/prototypes/__init__.py:30:1: F403 'from django.contrib.gis.geos.prototypes.topology import *' used; unable to detect undefined names
./django/contrib/gis/geos/tests/test_geos_mutation.py:11:1: F403 'from None import *' used; unable to detect undefined names
./django/contrib/gis/db/backends/mysql/base.py:1:1: F403 'from django.db.backends.mysql.base import *' used; unable to detect undefined names
./django/contrib/gis/db/backends/postgis/base.py:1:1: F403 'from django.db.backends.postgresql_psycopg2.base import *' used; unable to detect undefined names
./django/contrib/gis/db/backends/oracle/base.py:1:1: F403 'from django.db.backends.oracle.base import *' used; unable to detect undefined names
./django/contrib/gis/db/models/__init__.py:2:1: F403 'from django.db.models import *' used; unable to detect undefined names
./django/contrib/gis/db/models/__init__.py:5:1: F403 'from django.contrib.gis.db.models.aggregates import *' used; unable to detect undefined names
./django/contrib/gis/db/models/sql/aggregates.py:1:1: F403 'from django.db.models.sql.aggregates import *' used; unable to detect undefined names
./django/contrib/messages/__init__.py:1:1: F403 'from django.contrib.messages.api import *' used; unable to detect undefined names
./django/contrib/messages/__init__.py:2:1: F403 'from django.contrib.messages.constants import *' used; unable to detect undefined names

Change History (8)

comment:1 Changed 20 months ago by loic84

It's very likely that this is going to break some people's code, so it would be useful to add a mention in the backward incompatible changes section of the release notes.

comment:2 Changed 20 months ago by bmispelon

  • Cc bmispelon@… added
  • Triage Stage changed from Unreviewed to Accepted

+1 for fixing this.

Some of the star-imported modules define __al__ so it's not that bad but that's not the case for all in this list so there's definitely room for some cleanup.

As for backwards-compatibility, I think provided we don't remove documented stuff we should be good with just a mention in the release notes.

comment:3 Changed 19 months ago by timo

  • Owner changed from nobody to timo
  • Status changed from new to assigned

Current proposal as noted in this pull request is to keep some import * statements and make them explicit something like:

from .migration import Migration
from . import operations
from .operations import *  # NOQA

__all__ = ['Migration'] + operations.__all__

Alternate proposals welcome, as I'm not too sure if this is the best solution.

comment:4 Changed 19 months ago by anonymous

I suggest make it a warning, not an error.
error is too annoying.

comment:5 Changed 19 months ago by akaariai

For backwards compatibility - I also vote that supporting documented imports is enough. If there are common cases that cause problems, then we can always expand what is documented.

comment:6 Changed 19 months ago by timo

  • Has patch set

PR is ready for review. It also removes unused imports across the code base.

comment:7 Changed 19 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 36ded01527b690b5df0574492af9cfcc2ea3d1dc:

Fixed #21302 -- Fixed unused imports and import *.

comment:8 Changed 19 months ago by Florian Apolloner <florian@…>

Note: See TracTickets for help on using tickets.
Back to Top