pylint-django

pylint-django copied to clipboard

A user just asked a question on the Django forum about why pylint says to use get_user_model().

Here's my response:

Django does say to use get_user_model()… only when building reusable apps - i.e. those intended for use in multiple projects, installed via PyPI or similar.

This is covered under the heading Reusable apps and AUTH_USER_MODEL in the docs.

pylint-django is somewhat mistaken in always recommending this. If you are building a normal Django project you do not need to use get_user_model().

The Django docs could perhaps be reworded to reiterate this.

Perhaps you could activate this check only with an extra flag, that says users are checking a reusable app.

If you are building a normal Django project you do not need to use get_user_model().

I think you are also slightly mistaken in this statement. I've contributed this checker from a different open source project, which is a standard web application. We use the standard User model but instead of referencing it directly we go through get_user_model(). All these years that's not been strictly necessary and complying with the best practices has been seen as annoying by some developers on our team.

Right now it looks like we have new functional requirements which are likely to necessitate the switch to a custom user model. Having code which uses get_user_model() everywhere will make this switch very easy and without the risk of introducing bugs and without the need to recheck all of the places in the code where user models are used.

If this what most users want - probably not. But pylint itself provides a lot of extra checks which are considered good practice. I would argue that pylint-django is doing the same in this example.

It is also very easy to disable pylint checks globally, either via config files or cli params so users could opt out of it if they don't like it or really know what they are doing. My opinion is that all linter checks should be enabled by default (when technically possible) and let users disable the ones they don't like instead of opting-in into some nice checkers.

@carlio thoughts ?

I agree with @atodorov .

Firstly, an extra flag would just be a special way to disable or enable a single error message, and it doesn't seem necessary to add a custom feature when it's already possible to turn it off.

Secondly, and most importantly, from the Django documentation:

If you’re starting a new project, it’s highly recommended to set up a custom user model, even if the default User model is sufficient for you.

As @atodorov says, pylint is not only about finding errors but about recommending best practices too - it'll raise consider-using-f-string if I write x = "something %s" % y

Given that the word "recommended" appears in the documentation about new projects then I think it's valid to raise that from pylint-django and have people turn it off if they choose, rather than making people turn it on. get_user_model() isn't simply about writing a reusable app.

Secondly, and most importantly, from the Django documentation:

If you’re starting a new project, it’s highly recommended to set up a custom user model, even if the default User model is sufficient for you.

Yes, it's a good idea to use a custom user model. This does not influence whether you should use get_user_model() to import it. You don't need to use get_user_model() unless you are building a reusable app.

get_user_model() isn't simply about writing a reusable app.

It is though. It's only documented under the "reusable apps" heading on purpose.

And, if you check the implementation, you'll see it's basically an import via a reference to a setting:

https://github.com/django/django/blob/c121e32082edaff817d9f69dec0c24855dc0186f/django/contrib/auth/init.py#L165-L179

Within a project, you can just perform that import directly. This is clearer and allows IDE's and even pylint itself to follow the import.

Sure, if you swap your user model you'll need to update the imports. That's just like any other migration though. There's no need to use the function "just in case" you change user model, which is something projects normally do 0 or 1 times.

---

Yes users can just turn off this check, but they have to know to do so. If it's not worth adding a flag, then perhaps you can make the check opt-in rather than opt-out. I'm afraid I don't know pylint very well as I don't use it often.

You don't need[emphasis mine] to use get_user_model() I'm afraid I don't know pylint very well as I don't use it often.

I think this is where the difference of opinion is - pylint does raise several things which are more about "eating your vegetables" as well as things which prevent your code working. As the example above, x = "something %s" % y is absolutely functional but pylint will still suggest using f strings in places of it.

In that sense, this warning from pylint-django is exactly on-brand with how pylint operates - warn or notify about everything and let users filter out what they don't want. It'd be inconsistent with pylint to switch to 'optional on' instead of 'optional off'

While I totally understand that imported-auth-usuer isn't relevant to everyone, it's also definitely a recommendation even for a new project in the documentation and easily turned off.

For that reason, adding a # pylint: disable=imported-auth-user in the relevant module, or adding to a .pylintrc or other ways of customising pyling is in my opinion the way to go, because that allows pylint-django to be an extension of the same behaviour and not include any surprises by asking you to turn on a warning.

Maybe if imported-auth-user is meant to be a recommendation, the message should be a Convention or Warning rather than an Error. Errors are usually things the linter thinks will make the code not run, like non-existent imports. Some code that imports the auth User model might not run, but lots will. To me, that suggests it should be a Warning.

A different level sounds like it could be a good compromise.

I forgot to share here that I wrote a post on the subject: https://adamj.eu/tech/2022/03/27/you-probably-dont-need-djangos-get-user-model/ (not really any new info)