[GSOC 2024] Auto-importing in the shell

[salvo-polizzi]

Hi everyone!
I’m very entusiast that my proposal for GSOC’24 has been accepted! Thanks to @DevilsAutumn for feedback about a possible implementation of this feature and for helping me in writing the proposal.

This topic will be the one where I will update the progress of the PR: so every feedback is welcome. I’m very excited and entusiastic about this project and I’m looking forward to implement the auto-import for the shell.

Possible useful links:

• Proposal
• First Trial

In the coming days I will create a new PR that is going to be the one where the major part of the work will be done. In the mean time, if anyone wants to say something about this approach or the implementation described in the proposal, I’m here to listen all your feedback and I’ll be happy to review my work with your help.

Thanks,
Salvo Polizzi

[DevilsAutumn]

Congrats @salvo-polizzi ! I’m really excited to work with you.
Just to be clear on what our final Shell will look like, I want to note down all the functionalities of shell_plus that we plan to add in Django Shell here. This way we can get to know the views of the community as well.

1. By default, we’ll not print all model names in the Shell but it would be possible with a flag.
2. Adding SHELL_PLUS_IMPORTS settings, see: shell_plus — django-extensions 3.2.3 documentation
3. Adding SHELL_PLUS_PRINT_SQL settings, see: shell_plus — django-extensions 3.2.3 documentation
4. Import all models using their full paths and then import the models in the order of the app names, starting with the first encountered app.
5. User should be able to subclass the Shell command and override a method.
6. By default, Django supports IPython, BPython, and standard shell runners. I believe we should enhance the current shell to allow users to add additional shell runners (jupyter notebook, ptpython etc) while preserving the auto-import functionality across all runners.

[adamchainz]

Hi @salvo-polizzi , congratulations on being accepted. I proposed this project last year and I’m glad to see it is being picked up. I am not your mentor because I failed to log onto the GSoC panel, but I’m on hand to review your PRs.

Please review the past discussion with another potential candidate: Have Solved 80% of auto_importing Shell feature , and their PR: [gsoc] django_auto_import shell feature by Ammar-Munirr · Pull Request #17826 · django/django · GitHub .

In terms of features, I propose we don’t directly copy shell_plus. Instead, we should try to build a minimal, extensible design.

django-extensions’ design has “grown by committee” (or anarchy). Pretty much all PRs were merged, meaning its features are pretty haphazard. Its code shows that internally, with untested blocks, “secret” undocumented features, and general messiness. We’d rather make fewer solid features in Django, so we can maintain them for a long time.

Here’s how I’d refine Bhuvnesh’s suggestions:

1. The shell command gains a new method, called, say, get_namespace(). This method returns a dictionary of names to objects, which are all added into the namespace.

2. The default implementation of get_namespace() adds all models from apps.get_models(). We can have some debate about what to do with name collisions. But whatever we do, models in earlier apps need to take precedence, as that’s the precedence order used elsewhere, such as for management command lookup.

3. The SHELL_PLUS_IMPORTS is not needed. Instead, we document how to extend get_namespace() to add other imports, something like:

# myapp/management/commands/shell.py
from django.core.management.commands import shell

class Command(shell.Command):
    def get_namespace(self):
        from django.core.urls import resolve, reverse
        
        return {
            **super().get_namespace(),
            "resolve": resolve,
            "reverse": reverse,
        }

4. No feature to “display all imported objects”, nor an option to turn it on. I think we should stick with a simple message like “X objects automatically imported.”. Users can use globals() or shell-specific features to see all imported objects, if and when they care.

5. Copying the functionality from SHELL_PLUS_PRINT_SQL is out of scope.

6. This functionality should work in the supported shells as much as possible.

7. There’s no need to add support for extending with other shells, as that already exists. One can override the command with a subclass and add an extra method with its name in the shells attribute. But this is undocumented, so it would be a nice bonus to make a side PR documenting how to use this feature.

Thanks again for working on this project, I’m excited to see how it goes!

[salvo-polizzi]

Thanks @adamchainz for your valuable feedback, it clears also my mind about what to do. I also want to inform the community that I’ve created a draft PR where all the work will be done.

[adamchainz]

One other point @DevilsAutumn messaged me on Discord: we should make sure the default imports are loaded for -c / --command as well, which bypasses launching a shell interface by using exec():

django/django/blob/c0b0ce85ede1f1baf8826fb6c10c23f9adcbcca3/django/core/management/commands/shell.py#L116-L118

The globals() argument can be expanded. That would allow quick one liners like:

$ ./manage.py shell -c 'print(User.objects.count())'
9001

[adamchainz]

Oh, yeah, we should also make sure the default imports are loaded for the stdin feature too, the block below the -c one.

[salvo-polizzi]

This is how the import of the objects is currently displayed, following the advice of @adamchainz. What do you think?

[salvo-polizzi]

Hi folks!

I’m here with a little update about the work we are currently doing.

There’s an ongoing discussion about how to handle name collisions. The approach we are considering is to give precedence to models in the earlier apps listed in INSTALLED_APPS. Additionally, we are thinking of providing a way to access overridden models. @adamchainz suggested importing all model modules as <app_label>_models and then accessing these overridden models by <app_label>_models.Model, for example, auth_models.User.

If anyone has any ideas or would like to suggest a strategy to adopt for handling name collisions, I’d be happy to hear them.

[DevilsAutumn]

Hi, One thing that I still feel would be better to add is the SHELL_IMPORTS settings. I see the the shell customization on 2 levels.

At level 1, users would just want to import extra modules, submodules, methods etc. And I think this will be the most common use case.
At level 2, users would want to perform some kind of manipulation or do some pre-processing on their data. This one would be comparatively uncommon.

At this point, to achieve both levels, overriding get_namespace is the only option. But with SHELL_IMPORTS settings, we can super simplify things for atleast users at level 1(where most of the users lie?). As a developer, I would not want to create a bunch of files and folders, subclass a command and override a method just to import few methods/submodules/modules for testing. Rather I would prefer to write 3-4 lines in the settings.py and expect django to handle the complications of importing them.

Yes, If I had some complex operations to perform before launching the shell or want to use some other python runner, then I would definitely have no problem to subclass the shell command and override a method.

Would be happy to know the opinions of the community on this.

[sarahboyce]

For those who are curious of this feature, please see:

• #35515 (Auto-importing models feature for shell-command) – Django
• https://github.com/django/django/pull/18158

Testing and feedback is always welcome

[nessita]

Hello everyone!

Thank you @salvo-polizzi for working on this. I’m in the process of reviewing the PR and it looks great. Thanks also to @adamchainz and @DevilsAutumn for mentoring this feature!

I know I may be a bit late to the discussion (I couldn’t engage earlier – sorry about that), but I wanted to share some thoughts about the items that are automatically imported. This feedback comes after using and reviewing this work. Above all, I want to respect the previous consensus, but I also want to bring the following to the table for further consideration:

I would prefer not to have the automatic import of the models module for each INSTALLED_APPS aliased to appname_models.

My understanding is that this was added to help with cases where colliding model names could potentially cause some models to be unavailable in the shell. However, I think that in everyday Django work, having:

• colliding model names
• that are needed in the console
• during (likely) debugging

feels like a minority of cases. A Django user can always manually import it with something like from myapp.models import CollidingName as CollidingNameNewer.

The downside of the current approach, where these models are always imported with an alias, is that it clutters the namespace and disrupts the clean and beatiful printing of the imported modules in the shell’s “new preamble”.

To exemplify my point, this is the current proposal (run in a very small pet project of mine, with isort installed):

But I rather something more in line with this:

As a reference, shell_plus (by default) does the following:

(Name collisions in shell_plus are handled via collision resolvers, I don’t think we need such a complex mechanism either.)

Lastly, to further support my proposal, I’ll quote @carltongibson from this post:

Simple, to the point, covers the 80% case is probably enough for an addition to Django itself here, leaving space in the ecosystem for folks to explore more advanced options.

Thanks everyone!

[carltongibson]

In all my time using Django, I can count the number of times I’ve needed… let’s see… the Session model in a shell session on one hand. User, OK often. ContentType only occasionally.

Having to create a custom management command to override this (as per docs in PR) seems quite heavy-weight.

TBH my preference here would be to import ≈nothing by default, and give users a declarative override point. (My own usage is to import just the things I need for that session. I don’t want everything imported, especially when using auto reloading, in iPython say.)

@DevilsAutumn

As a developer, I would not want to create a bunch of files and folders, subclass a command and override a method just to import few methods/submodules/modules for testing. Rather I would prefer to write 3-4 lines in the settings.py and expect django to handle the complications of importing them.

This seems along a better lines. Could the get_namespace() function be referenced by a setting (declared in settings.py ideally) if it’s not feasible to make it a purely text based list of imports?

Edit: something like a —no-imports flag also seems needed. (I might have missed that.)

[Lily-Foote]

A possible middle ground here for import collisions would be to use your approach as a default, but have a documented flag for importing the aliased modules.

That said, perhaps the maintenance and documentation complexity isn’t even worth this and we can just let users manually import when there is a clash.

[carltongibson]

As I was pondering this this afternoon, I wondered if the cool thing to make available wouldn’t be apps.get_model() since then any model is just a get_model(‘auth’, ‘User’) away. (But likely that’s not what folks are looking for.)

[CodenameTim]

@nessita

I would prefer not to have the automatic import of the models module for each INSTALLED_APPS aliased to appname_models.

It appears that the aliased imports occur only when there is a model name collision. However, I’m guessing whenever one app imports a model from another app, that’s causing a collision (@salvo-polizzi is this a bug?). Since the collisions are likely to be common then and we have a pattern for how they are imported (f"{app_name}_models"), perhaps we don’t bother logging them at all as Adam suggested earlier:

@adamchainz

4. No feature to “display all imported objects”, nor an option to turn it on. I think we should stick with a simple message like “X objects automatically imported.”. Users can use globals() or shell-specific features to see all imported objects, if and when they care.

I think the benefit of being told which models are being imported is very minimal. Perhaps the first few times it’s helpful, but on my 10,000th time using shell_plus, the list of 200 models is simply taking up scroll space in my terminal.

---

Regarding how to override the basic default functionality, I agree with @DevilsAutumn and @carltongibson that there should be a lighter approach than defining a management command.

[salvo-polizzi]

Hi everyone,

It’s great to hear feedback from the community! I’d like to share my thoughts on the discussion so far.

@nessita

I would prefer not to have the automatic import of the models module for each INSTALLED_APPS aliased to appname_models.

That’s a valid point. Originally, as you mentioned, the feature was introduced to handle collisions. However, I agree that it’s straightforward for users to manage collisions themselves by importing models explicitly and defining their own aliases.

@carltongibson

TBH my preference here would be to import ≈nothing by default, and give users a declarative override point.

The core idea behind this feature was to automatically import models from apps while giving users the ability to customize this behavior. I believe the current implementation achieves this fairly easily, as outlined in the accompanying tutorial. That said, if there’s a consensus to move towards a different approach, I’d be open to exploring alternatives.

@CodenameTim

However, I’m guessing whenever one app imports a model from another app, that’s causing a collision (@salvo-polizzi is this a bug?)

I hadn’t considered this issue, so I’ll need to test it further. Basically what get_namespace() do is to loop over all models, using apps.get_models(), and when models with the same name exist in the namespace, the first one listed in INSTALLED_APPS takes precedence and overrides any subsequent ones.

@CodenameTim

I think the benefit of being told which models are being imported is very minimal. Perhaps the first few times it’s helpful, but on my 10,000th time using shell_plus, the list of 200 models is simply taking up scroll space in my terminal.

This is why we decided to show the imports only when verbosity=2. I agree that the output can become excessive, but since it’s an optional flag, I think it’s still helpful for users who are new to a project. It provides useful context during the initial setup. However, I’m open to discussing whether this feature should be reworked or removed if there’s strong consensus.

[carltongibson]

@salvo-polizzi

The core idea behind this feature was to automatically import models from apps while giving users the ability to customize this behavior. I believe the current implementation achieves this fairly easily, as outlined in the accompanying tutorial.

Yes, and that was point of the project, agreed

Just two things:

1. There needs to be an off switch of some kind, otherwise I can’t get a clean shell, without bootstrapping Django myself. IIUC there isn’t yet one in the PR.
2. As per my comment, I think the override the management command approach offered for customisation is too heavy-weight. I think there needs to be something lighter.

[carltongibson]

The configuration options for Django extensions’ shell_plus look to be exactly in line with what’s needed here.

Sometimes, models from your own apps and other people’s apps have colliding names, or you may want to completely skip loading an app’s models. Here are some examples of how to do that.

Note: These settings are only used inside shell_plus and will not affect your environment.

# Rename the automatic loaded module Messages in the app blog to blog_messages. 
SHELL_PLUS_MODEL_ALIASES = {'blog': {'Messages': 'blog_messages'},} 
# Prefix all automatically loaded models in the app blog with myblog. 
SHELL_PLUS_APP_PREFIXES = {'blog': 'myblog',} 
# Dont load the 'sites' app, and skip the model 'pictures' in the app 'blog' 
SHELL_PLUS_DONT_LOAD = ['sites', 'blog.pictures'] 
# Dont load any models 
SHELL_PLUS_DONT_LOAD = ['*'] 

You can also combine model_aliases and dont_load. When referencing nested modules, e.g. somepackage.someapp.models.somemodel, omit the package name and the reference to models. For example:

SHELL_PLUS_DONT_LOAD = ['someapp.somemodel', ]  # This works 
SHELL_PLUS_DONT_LOAD = ['somepackage.someapp.models.somemodel', ]  # This does NOT work 

It is possible to ignore autoloaded modules when using manage.py, like:

$ ./manage.py shell_plus --dont-load app1 --dont-load app2.module1 

Command line parameters and settings in the configuration file are merged, so you can safely append modules to ignore from the commandline for one-time usage.

[nessita]

Thanks everyone for your responses, and once again thank you @salvo-polizzi for your openness to make further changes.

In the interest of time and with a sincere goal to merge this feature for 5.2, I propose the following plan. I considered all feedback, including my own as Django user and also as a Django gatekeeper.

1. Moved out the functionality that automatically adds to the namespace the models modules with an alias. Remove this from examples docs. Potentially make it a separated commit/PR with an optional CLI flag.
2. A new flag is added and documented to avoid auto imports altogether (--no-imports).
3. Tweak the informative output so:
   2.1. -v 0 nothing extra is printed (no change)
   2.2. -v 1 prints Automatic imports: N objects imported (use -v 2 for details).
   2.3. -v 2 prints

Automatic imports: N objects imported.
    ModelA
    ModelB
    ModelC

For now I think is better to not add any new settings, until further and more precise feedback is received from the wider Django users community.