Opened 3 weeks ago
Closed 2 weeks ago
#35866 closed Cleanup/optimization (fixed)
Django documentaion style guide on models is unclear what to do with any other Python dunder methods that the model class might have
Reported by: | Hristo Trendafilov | Owned by: | Hristo Trendafilov |
---|---|---|---|
Component: | Documentation | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Micha Reiser | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Following the discussion here:
https://github.com/astral-sh/ruff/issues/13892#issuecomment-2436995567
It turns out that the Django documentation regarding model structuring https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/#model-style is unclear.
As proposed:
The order of model inner classes and standard methods should be as follows (noting that these are not all required): All database fields Custom manager attributes class Meta def __str__() def save() def get_absolute_url() Any custom methods
makes understanding that any other dunder, private methods and so on should come after the methods mentioned above,
which violates Python class best practices of structuring code in classes, where dunder methods should be on top, then constants, staticmethods, private methods and so on...
The following code is an example:
What Python best practices suggests:
class Person(models.Model): name = models.CharField(xxxx) def __repr__(self): return "something" def save(*args, **kwargs): super().save(*args, `**kwargs)
What Django documentation suggests:
class Person(models.Model): name = models.CharField(xxxx) # Django methods def save(*args, **kwargs): super().save(*args, `**kwargs) # Any custom methods def __repr__(self): return "something"
Change History (8)
comment:1 by , 3 weeks ago
Cc: | added |
---|
follow-up: 3 comment:2 by , 3 weeks ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Cleanup/optimization |
Version: | → dev |
comment:3 by , 3 weeks ago
Replying to Natalia Bidart:
Hello Hristo Trendafilov, thank you for taking the time to create this report. To me, "any custom method" is a method that you "invent" for your Model, not methods that are already "defined" by Python's base object class.
So my reading of that is that whatever extra business logic and helpers that you need are after
get_absolute_url
. I agree we could make this explicit in the docs though, accepting with that goal.
Would you like to prepare a patch?
Hello, I have been thinking about that and my proposal is like so:
- All database fields
- Custom manager attributes
- class Meta
- any dunder and magic method(s), except
__str__()
def __str__()
- any
@property
/ies/ + setter/s/ - any
@classmethod
/s/ - any
@staticmethod
/s/ def save()
def get_absolute_url()
- Any other public methods
- Any other private methods
The reason for this is like so:
- Developers are keen on reusing methods as fast as possible, so important things you might ever want to use or overwrite should be at the top, to avoid scrolling.
- Properties might be considered as fields/Django Admin even allow you to use those/, so those should be up but after the
__
methods and magic methods to keep Python best practices for class ordering __str__()
to be the last dunder, to conclude the dunder block, so that you know that there will be no other__
after that point. Also in rare cases__init__()
is used that might go to the top of the dunder block.- Class methods and static methods should be at the top because they could be widely used when a class is inherited or initialized and to avoid scrolling.
def save()
anddef get_absolute_url()
could be an anchor telling you that from this point you might add your custom logic. It will be easier to read because you will know that after this point there is some custom logic.- Finally, to avoid unnecessary scrolling, private methods should be at the bottom, because most likely you will not need or want to look at those.
If that is agreed upon, I am okay with fixing it.
comment:4 by , 3 weeks ago
Has patch: | set |
---|---|
Owner: | set to |
Status: | new → assigned |
comment:5 by , 2 weeks ago
Patch needs improvement: | set |
---|
comment:6 by , 2 weeks ago
Patch needs improvement: | unset |
---|
comment:7 by , 2 weeks ago
Triage Stage: | Accepted → Ready for checkin |
---|
Hello Hristo Trendafilov, thank you for taking the time to create this report. To me, "any custom method" is a method that you "invent" for your Model, not methods that are already "defined" by Python's base object class.
So my reading of that is that whatever extra business logic and helpers that you need are after
get_absolute_url
. I agree we could make this explicit in the docs though, accepting with that goal.Would you like to prepare a patch?