Code

Opened 4 years ago

Last modified 3 days ago

#14638 new Cleanup/optimization

to_python howto documentation improvement

Reported by: maraujop Owned by: nobody
Component: Documentation Version: master
Severity: Normal Keywords: to_python, model, field
Cc: muchochini@…, anubhav9042@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Reading this:
http://docs.djangoproject.com/en/dev/howto/custom-model-fields/#django.db.models.to_python

Anyone would think that to_python is a model method in charge of converting a value extracted from a DB to a python type. Beforehand in the documentation there is a way better explanation of this method.

http://docs.djangoproject.com/en/dev/howto/custom-model-fields/#modelforms-and-custom-fields

I honestly think it should be redone, I lost ages trying to figure out what this method was for.

Thanks, regards
Miguel Araujo

Attachments (1)

14638.diff (1.6 KB) - added by anubhav9042 3 days ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 4 years ago by gabrielhurley

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from to_python documentation is not comprehensible to to_python howto documentation improvement
  • Triage Stage changed from Unreviewed to Accepted

On the one hand, the docs are correct. When not using SubfieldBase, the to_python() method gets called (primarily) to convert data returned by the DB into the appropriate Python type. It is only when you're using SubfieldBase that to_python() is called on every assignment of a value to the field. So the split in the documentation is there for a reason, and neither description is wrong.

On the other hand, I will agree that if you are googling for what to_python() does you'll likely land on the latter section first, and offering a more general introduction to the method and it's various possible uses before launching into the details of its most common use would be helpful.

comment:2 Changed 4 years ago by maraujop

  • Cc muchochini@… added

I don't think docs are correct or I'm missing something. I have tried different subclasses and in all of them to_python gets called before assigning a value to the field. to_python is the method in charge of converting the value to the expected python type to see if it can be assigned.

I have put prints in my own DecimalField to see when they were being called. I haven't seen to_pỳthon been called to convert data returned by the DB never. Here is the code of my subclass:

class Test(models.DecimalField):
    def to_python(self, value):
        print "to_python"
        if value is None:
            return value
        try:
            return decimal.Decimal(value)

        except decimal.InvalidOperation:
            raise exceptions.ValidationError(self.error_messages['invalid'])

    def __unicode__(self):
        print "__unicode__"
        return self.to_python()

The code is the same used in DecimalField. So I'm doing something wrong or to_python does not work as the docs say.

Thanks, regards

comment:3 Changed 4 years ago by gabrielhurley

So I have an answer for your question, but not a right answer for the documentation... here's the answer to your question:

You are not seeing to_python called because you are subclassing DecimalField. For one, all of the standard Django field types subclass Field, which *does not* subclass SubfieldBase (which guarantees to_python being called). Second, DecimalField registers a separate database conversion callback function for sqlite (and others). These types of functions exist to handle the vagaries of the different backends. There are a handful of fields that use this behavior, but which fields may vary by backend. Since they use custom conversion functions (django.db.backends.util.typecast_decimal in this case) they bypass to_python on the return trip from the db. to_python *is* called during validation on the field (such as when saving the model).

Effectively, if you are subclassing existing fields, to_python is not guaranteed to be called, and that behavior may be backend-dependent. If you want it to be called consistently, you should do as the docs say, and subclass SubfieldBase.

There is a takeaway message from all this that should be encapsulated in the docs, but at present I'm not quite sure what that message is. Given that this is a how-to doc, and SubfieldBase is addressed earlier in the doc I'm inclined to say that the docs should simply stress subclassing SubfieldBase more heavily, with a little more warning about subclassing existing fields...

Input from others is welcome.

comment:4 Changed 4 years ago by maraujop

Thanks gabrielhurley,

That makes sense. I have changed my code and now to_python is being called as I was expecting. I have to say I've lost a lot of hours trying to figure this out and the documentation could be much clear on this. When I was asking this online, stackoverflow and IRC, people were answering to_python (that's what I thought before asking the first time), but I was saying, no, it's not being called.

Beware that you don't subclass SubfieldBase . SubfieldBase is a metaclass. So now I'm subclassing DecimalField with SubfieldBase as a metaclass and It works as I wanted.

I totally agree that the docs should stress this more heavily, but I also think some parts should be rewritten to be more coherent. I will make a quote from the docs as an example:

If you use SubfieldBase, to_python() will be called every time an instance of the field is assigned a value. This means that whenever a value may be assigned to the field, you need to ensure that it will be of the correct datatype, or that you handle any exceptions.

This is not true. If you use SubfieldBase to_python will be called when assigning a value and when getting the value from the database.

Converts a value as returned by your database (or a serializer) to a Python object.

True, but if only you are using SubfieldBase and not completely true, as it does more than that as you know.

Thank you very much for your help, I had already desisted on this
Miguel Araujo

comment:5 Changed 3 years ago by julien

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:6 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:7 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:9 Changed 4 weeks ago by timo

@jorgecarleitao, did you read the comments on this ticket? The report is describing issues with those docs you linked -- there hasn't been anything added since this report as far as I've seen.

comment:10 Changed 3 days ago by anubhav9042

With better documentation in e1fa7df, I think now the various usage of to_python is clear now.
IMO we can close this now.

comment:11 Changed 3 days ago by anubhav9042

  • Cc anubhav9042@… added

comment:12 follow-up: Changed 3 days ago by timo

Have you read the comments above and taken time to understand the nuances (I haven't)? For example, in comment 4 it says: "I will make a quote from the docs as an example: [quote] This is not true." I am not sure if the report is correct or not, but that paragraph remains unaltered even after the commit you mentioned.

comment:13 in reply to: ↑ 12 Changed 3 days ago by anubhav9042

Replying to timo:
Yes I have gone though the docs and understood the situation.
Earlier the entire docs for to_python was in custom_field docs but now it is not. It has been moved to appropriate place where it mentions its usual purpose of Converts a value as returned by the database (or a serializer) to a Python object. and also mentions See converting-database-values-to-python-objects for usage., see here, where it mentions about its extra purpose of being used with SubFieldBase and this same is said in the docs for SubFieldBase.

I know that lines haven't been changed, but I felt that this way of representation cleared everything. Whatever the doubt was, it was about the two different usages of to_python which is surely mentioned under proper headings now, so I don't think anything else is required.

At max, we can change the link

See converting-database-values-to-python-objects for usage.

to

See converting-database-values-to-python-objects for various usages.

and

If your custom field needs the to_python method to be called when it is created, you should be using The SubfieldBase metaclass mentioned earlier.

to

If your custom field needs the to_python method to be called everytime it is created along with its usual usage, you should be using The SubfieldBase metaclass mentioned earlier.
Last edited 3 days ago by anubhav9042 (previous) (diff)

comment:14 Changed 3 days ago by anubhav9042

What do you suggest?

Changed 3 days ago by anubhav9042

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.