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 Micha Reiser, 3 weeks ago

Cc: Micha Reiser added

comment:2 by Natalia Bidart, 3 weeks ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization
Version: dev

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?

in reply to:  2 comment:3 by Hristo Trendafilov, 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() and def 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 Hristo Trendafilov, 3 weeks ago

Has patch: set
Owner: set to Hristo Trendafilov
Status: newassigned

comment:5 by Sarah Boyce, 2 weeks ago

Patch needs improvement: set

comment:6 by Sarah Boyce, 2 weeks ago

Patch needs improvement: unset

comment:7 by Sarah Boyce, 2 weeks ago

Triage Stage: AcceptedReady for checkin

comment:8 by Sarah Boyce <42296566+sarahboyce@…>, 2 weeks ago

Resolution: fixed
Status: assignedclosed

In b50d1a0:

Fixed #35866 -- Clarified the positioning Python magic methods on models in the internal style guide.

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