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:
- Document the behavior, maybe here: https://docs.djangoproject.com/en/5.2/howto/custom-shell/ ; and/or
- Emit a warning in this case; and/or
- 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 , 17 months ago
comment:2 by , 17 months ago
| Summary: | Identically named models in different apps → Improve handling of model name collisions in shell auto-imports |
|---|
comment:3 by , 17 months ago
| Cc: | 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 , 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 , 16 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:6 by , 16 months ago
| Has patch: | set |
|---|---|
| Patch needs improvement: | set |
comment:7 by , 3 weeks ago
| Owner: | changed from to |
|---|
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.
follow-up: 11 comment:8 by , 12 days ago
Can someone give me some insight into why isort is even invoked?
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 , 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 , 5 days ago
| Patch needs improvement: | unset |
|---|
comment:11 by , 3 days ago
Replying to john-parton:
Can someone give me some insight into why
isortis even invoked?
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 , 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.
Thanks for taking the time to create this ticket.
I can reproduce this, but for me this is expected behavior, as documented here:
That said, the note could benefit from improved wording to make the meaning more explicit. E.g.
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 byisortand 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.