Code

Opened 2 months ago

Last modified 6 weeks ago

#21940 new Cleanup/optimization

Consistent contribute_to_class's virtual_only

Reported by: Kronuz Owned by: nobody
Component: Database layer (models, ORM) Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I needed to create a field like class for my django project and I wanted it to be a virtual field... virtual_only is there for that reason, but my field was inheriting from FileField, and FileField.contribute_to_class() doesn't take virtual_only and so it doesn't passes it to Field.contribute_to_class(). I believe passing virtual_only attribute should be consistent across the source code. I'm attaching a simple patch to fix this. One warning would be for custom fields made by users which have their own contribute_to_class (those will need to be changed to properly receive the virtual_only parameter as optional.)

Attachments (1)

#21940-virtual_only_in_contribute_to_class.diff (4.1 KB) - added by Kronuz 2 months ago.

Download all attachments as: .zip

Change History (4)

comment:1 Changed 2 months ago by timo

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Would it be better to simply accept and pass on **kwargs? I think that would eliminate the minor backwards compatibility concerns as well.

comment:2 Changed 2 months ago by timo

  • Component changed from Uncategorized to Database layer (models, ORM)
  • Has patch set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

comment:3 Changed 6 weeks ago by anubhav9042

IMO what is done is better, because

In the contribute_to_class in Field class:

def contribute_to_class(self, cls, name, virtual_only=False)

If from subclass:

super(AutoField, self).contribute_to_class(cls, name, **kwargs)

There is only virtual_only as keyword argument, if we pass **kwargs then it will be possible to pass any other variable to it as well, which is wrong I believe.

Last edited 6 weeks ago by anubhav9042 (previous) (diff)

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.