Code

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#13354 closed (invalid)

Possible bug, or possible docs error.

Reported by: orokusaki Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords:
Cc: orokusaki, kmtracey Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I wanted to use this to return empty strings for password fields, and $50.00 format for money that's stored as a decimal. You can imagine my surprise when I found out that I was trying to save $50.00 to a decimal field, and '' as somebody's password. The confusion arose from this:

In the following docs, it says that get_prep_value() is the reverse of to_python() (which would infer that get_prep_value() is used to prepare incoming data, and to_python() is used on outgoing data.

http://docs.djangoproject.com/en/dev/howto/custom-model-fields/#converting-python-objects-to-query-values

However, upon inspection of the code I found the following.

    def get_prep_value(self, value):
        return self.to_python(value)

They are in fact not the reverse of each other, but in some cases one in the same, since one uses the other for implementation.

I'm starting this as a ORM ticket rather than a docs ticket, only because with the above mentioned there doesn't appear to be a possible way to have a field output a modified value without also modifying it on the way in.

orokusaki

Attachments (0)

Change History (7)

comment:1 Changed 4 years ago by orokusaki

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

After reading this myself, I decided it wasn't clear what I was trying to do when I encountered this.

I'm trying to output an empty string so that when I display a password field, it is always empty. When the user submits the form, the password should not be emptied on the way back to the database. The same goes for formatting money so that users can see it in money format, but then when it's stored I would want to strip the dollar sign.

comment:2 Changed 4 years ago by kmtracey

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

Notice you are reading the documentation for Writing Custom Model Fields. This doc is attempting to describe what your implementation of these methods needs to do, when you are writing your own custom model field. These are methods used internally when retrieving data from the DB and sending it back: these are not methods that application-level code ought to be calling. The fact that a particular field's implementation of get_prep_value uses its to_python method does not make it wrong, it just means for that particular field type that makes sense.

Getting answers to your questions of how to handle password field display and display of monetary values would be better done by asking questions on #django IRC or the django-users mailing list. Neither of these tasks requires writing a custom model field, so I think you are looking at the wrong level for answers here.

comment:3 follow-up: Changed 4 years ago by orokusaki

  • Cc kmtracey added

@kmtracey

I'm not calling the code myself in my application. I understand how to write custom model fields (I have about 9 in my current project), and I'm not looking for free support. My concern is that the same method is being called to render the DB value out to its Python value (which then is output in my model form to the user) as well as to prep what the user inputs for the DB again. This is very incorrect and needs to be addressed in some way (either by changing the docs, or patching).

If you read my entire ticket before answering and closing my ticket next time, this miss-communication won't occur.

comment:4 Changed 4 years ago by orokusaki

One final note. You mentioned:

The fact that a particular field's implementation of get_prep_value uses its to_python...

This really matters because {{ to_python }} is provided in the docs as part of the public API to hook in your own functionality. It even gives specific details on how to ensure it's run each time. It's not simply a matter of behind-the-scenes implementation.

comment:5 in reply to: ↑ 3 ; follow-up: Changed 4 years ago by russellm

  • milestone 1.2 deleted

Replying to orokusaki:

My concern is that the same method is being called to render the DB value out to its Python value (which then is output in my model form to the user) as well as to prep what the user inputs for the DB again. This is very incorrect and needs to be addressed in some way (either by changing the docs, or patching).

It's not 'very incorrect' at all.

Firstly, you will note that the get_prep_value/to_python equivalence is only true on 5 of Django's builtin fields:CharField, DateField, DateTimeField, DecimalField and TimeField. The default implementation of both get_prep_value and to_python is "return value".

Secondly, you will note the following pattern:

  • CharField - whatever the user gives you, insert unicode. Whatever the database provides you, return unicode.
  • [Date|Time|DateTime]Field - whatever the user gives you, turn it into a date/time/datetime. Whatever the backend gives you (and there is some variation), turn it into a date/time/datetime.
  • DecimalField - whatever the user gives you, turn it into a Decimal. Whatever the database provides you, turn it into a Decimal.

That is, the equivalence of get_prep_value and to_python is entirely consistent with the desired behavior on those fields.

As for the original query - there certainly is a way to have a field output a modified value without also modifying it on the way in - do exactly what the documentation suggests, and define *both* methods.

comment:6 in reply to: ↑ 5 Changed 4 years ago by orokusaki

Replying to russellm:

Thanks for explaining why I was wrong. Sorry for wasting yet more time on hasty invalid conclusions. I still hold my position that the desired equivalent behavior would be best handled always in two separated methods, but to subclass and make a minor edit like that isn't the biggest deal.

I felt like all kmtracey did was pull out a straw hat.

comment:7 Changed 4 years ago by russellm

I don't think you are being fair to Karen at all. In fact, you could easily be accused of doing exactly the same thing that you have accused Karen of doing. Karen said:

The fact that a particular field's implementation of get_prep_value uses its to_python method does not make it wrong, it just means for that particular field type that makes sense.

Which is both correct, and consistent with what I said:

That is, the equivalence of get_prep_value and to_python is entirely consistent with the desired behavior on those fields.

Karen's comments say the same thing as I did -- I just gave more specific examples. That's not a "straw hat".

Karen also called into question whether you need a custom field at all. Based on the limited detail you provided, Karen couldn't see why a custom field is required -- and I can't either. What you seem to be describing is a set of basic data validation constraints, which is not the usual use case for a custom field that is reimplementing to_python() and get_prep_value(). The direction to seek out django-users was aimed -- correctly -- at encouraging you to seek out the right solution to the problem, rather than fixating on a complex way of solving the problem that may ultimately work but require a lot more effort. This isn't a "straw hat" either.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


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

 
Note: See TracTickets for help on using tickets.