Opened 17 months ago

Last modified 3 days ago

#36424 assigned Cleanup/optimization

Extend documentation related to handling of model name collisions in shell auto-imports

Reported by: john-parton Owned by: Alex Hatfield
Component: Core (Management commands) Version: 5.2
Severity: Normal Keywords:
Cc: Salvo Polizzi, Adam Johnson, Bhuvnesh Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

If you have two models with the same name, the automatic import logic will result in just one of them being imported.

$ ./manage.py shell -v2
116 objects imported automatically:

  from apps.order.models import Line, Order
  from apps.cart.models import Line, Cart

Python 3.13.3 (main, Apr  9 2025, 04:03:52) [Clang 20.1.0 ] on linux
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> Line
<class 'apps.cart.models.Line'>

shell_plus has a few different ways to resolve this issue: ​https://django-extensions.readthedocs.io/en/latest/shell_plus.html#configuration

In my settings.py, I have the following

SHELL_PLUS_MODEL_ALIASES = {
    "cart": {"Line": "CartLine"},
    "order": {"Line": "OrderLine"},
}

To make matters even more annoying, if you have isort installed, the verbose output doesn't even match the expected behavior of the last import being the ultimately imported symbol.

Output with isort

$ ./manage.py shell -v2
116 objects imported automatically:

  from apps.cart.models import Cart, Line
  from apps.order.models import Line, Order

Python 3.13.3 (main, Apr  9 2025, 04:03:52) [Clang 20.1.0 ] on linux
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> Line
<class 'apps.cart.models.Line'>

Note that you do not get a warning even if you run python with -Wall

I'm inclined to say that the behavior as-is could be considered a bug. From the perspective of the user, there's no explicable reason why one model is picked over the other and there's no indication that such an overwrite took place. However, this is basically the same behavior as shell_plus, so perhaps that's being too harsh with django.

Some possible fixes:

  1. Document the behavior, maybe here: ​https://docs.djangoproject.com/en/5.2/howto/custom-shell/ ; and/or
  2. Emit a warning in this case; and/or
  3. Don't import either model

Alternatively, implement user configurable "import as" or aliases, although that's outside the scope of a bugfix and into feature territory.

Change History (12)

comment:1 by Clifford Gama, 17 months ago

Thanks for taking the time to create this ticket.

I can reproduce this, but for me this is expected behavior, as documented ​here:

Models from apps listed earlier in INSTALLED_APPS take precedence.

That said, the note could benefit from improved wording to make the meaning more explicit. E.g.

If multiple apps define models with the same name, the model from the app listed earlier in INSTALLED_APPS will overwrite any previously imported model of the same name.

isort's role isn't documented and can easily confuse users trying to understand why one model was imported over another. I think we should add a note in the documentation and/or shell command help that the order shown may be affected by isort and may not the order actually used?

Leaving it unreviewed for now so others can weigh in on whether a warning or automatic resolution for name conflicts is worth pursuing.

comment:2 by Clifford Gama, 17 months ago

Summary: Identically named models in different apps → Improve handling of model name collisions in shell auto-imports

comment:3 by Natalia Bidart, 17 months ago

Cc: Salvo Polizzi Adam Johnson Bhuvnesh added
Summary: Improve handling of model name collisions in shell auto-imports → Extend documentation related to handling of model name collisions in shell auto-imports
Triage Stage: Unreviewed → Accepted
Type: Bug → Cleanup/optimization

Thank you John for your ticket, and thank you Clifford for the initial triage.

As Clifford says, this is documented and it was discussed at length in ​the forum post about this feature, and in the PR. This "priority" order also matches what is done in any other place where collisions can happen (management commands themselves, templates, translations, etc).

Documentation improvements are always welcome! We can accept the ticket on that basis. I would be open to consider a warning being shown, though that would have to be a 6.0-only feature due to the current translations string freeze that we have in effect for 5.2.

comment:4 by john-parton, 17 months ago

I'll see if I can improve the documentation slightly. My reading comprehension isn't the best, but I didn't realize that sentence was referring to this behavior.

Regarding warnings, I mostly meant a warning emitted if Python is run with -Wall. I don't believe those messages are internationalized, so my understanding is that it could be merged in before 6.0. Is that correct or am I mistaken?

(Aside: I'm of the opinion that if a user has to search the forums to figure something out, it's a sign that docs could be better.)

comment:5 by SOHAIL AHMAD, 16 months ago

Owner: set to SOHAIL AHMAD
Status: new → assigned

comment:6 by Clifford Gama, 16 months ago

Has patch: set
Patch needs improvement: set

comment:7 by Alex Hatfield, 3 weeks ago

Owner: changed from SOHAIL AHMAD to Alex Hatfield

Hi Sohail, since there hasn't been any activity on this ticket for a while, I'll reassign this to myself. If you're still working on this, please let me know.

comment:8 by john-parton, 12 days ago

Can someone give me some insight into why isort is even invoked?

​https://github.com/django/django/blob/9332b163a67eabe5bdbf8066b1c5da394be9aa9c/django/core/management/commands/shell.py#L243-L248

I searched the previously linked forum post and I don't see the word "sort" used anywhere in the discussion. Perhaps some discussion on a pull request?

Maybe we could just *not sort* them? I'm not exactly sure what value it really brings having them be sorted, as it appears to be just informational.

comment:9 by Alex Hatfield, 5 days ago

I've opened a ​new PR addressing this ticket, continuing the work of the ​previous PR.

My most recent comment on it, copied below, addresses the invocation of isort:

The ​original PR for the feature adding shell auto-imports ​suggests isort was used to align with the ​coding styles for handling imports. That is, to provide a clean and consistent shell output rather than any additional information.

My intent with this PR was to alert people to the issue as is as a first step. However, I do believe there is a fair question about whether isort should be invoked here, which I will follow up on in the Trac ticket.

I also couldn't find any discussion of using isort on the forum.

I agree not sorting them is an option, though I'm mindful of the consensus in the original PR to align with the existing coding style for imports. An alternative approach that keeps with this convention while respecting the prioritisation of models listed earlier in INSTALLED_APPS could be for Django to sort the imports itself, in almost exactly the same way isort would, differing only in that it explicitly accounts for app precedence. E.g. something like the below, where the comment mimics # isort:skip:

from apps.order.models import Line, Order
from apps.cart.models import Cart, Line  # out of sorted order: app precedence

Though this is a non-trivial piece of work compared to simply removing the isort ordering or a documentation change, and perhaps would deserve its own Trac ticket.

comment:10 by Alex Hatfield, 5 days ago

Patch needs improvement: unset

in reply to:  8 comment:11 by Natalia Bidart, 3 days ago

Replying to john-parton:

Can someone give me some insight into why isort is even invoked?

​https://github.com/django/django/blob/9332b163a67eabe5bdbf8066b1c5da394be9aa9c/django/core/management/commands/shell.py#L243-L248

I searched the previously linked forum post and I don't see the word "sort" used anywhere in the discussion. Perhaps some discussion on a pull request?

[duplicated from the PR comment]

We discussed this ​in the original PR. The idea was that, when available, isort would make the -v 2 listing follow Django’s usual import style. It was meant to format the listing, not to change import precedence or provide additional information.

Django generally aims to solve the 80% case with simple defaults. Models with the same name across apps are a less common case and can cause confusion beyond the shell listing.

comment:12 by Natalia Bidart, 3 days ago

Patch needs improvement: set

I have reviewed the most recent PR. I'd also be open to reviewing a patch that makes the -v 2 listing show only the imports whose names remain available in the shell after collisions are resolved. That would make the output accurate whether or not isort is installed.

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