Opened 5 years ago

Closed 12 months ago

Last modified 12 months ago

#14638 closed Cleanup/optimization (fixed)

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 13 months ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 5 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 5 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 5 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 5 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 4 years ago by julien

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

comment:6 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:7 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:9 Changed 14 months 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 13 months 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 13 months ago by anubhav9042

  • Cc anubhav9042@… added

comment:12 follow-up: Changed 13 months 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 13 months 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 13 months ago by anubhav9042 (previous) (diff)

comment:14 Changed 13 months ago by anubhav9042

What do you suggest?

Changed 13 months ago by anubhav9042

comment:15 Changed 12 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

In b6aa60f4252311ef8ef24b4ffd99984197be7ee6:

Fixed #14638 -- Clarified model Field.to_python() docs.

Thanks Anubhav Joshi for the patch.

comment:16 Changed 12 months ago by Tim Graham <timograham@…>

In d818e02134ae719108e12b24a7da7416328c2190:

[1.7.x] Fixed #14638 -- Clarified model Field.to_python() docs.

Thanks Anubhav Joshi for the patch.

Backport of b6aa60f425 from master

comment:17 Changed 12 months ago by Tim Graham <timograham@…>

In 955fdc8cdb0c1a98741687513980b64de8f20c75:

[1.6.x] Fixed #14638 -- Clarified model Field.to_python() docs.

Thanks Anubhav Joshi for the patch.

Backport of b6aa60f425 from master

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