Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#16291 closed Cleanup/optimization (fixed)

Document that TypedChoiceField does not coerce empty_value

Reported by: vanschelven Owned by: nobody
Component: Documentation Version: 1.3
Severity: Normal Keywords:
Cc: taavi223 Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

When the empty value is selected in a TypedChoiceField this is not passed through the coerce function.

I suppose this is what could be deduced from the existence of the empty_value parameter, but it was very surprising to me indeed.

Current description here:
https://docs.djangoproject.com/en/dev/ref/forms/fields/#typedchoicefield

My personal feeling would be to send all values through coerce, but I suppose the alternative would be to simply document this behavior more explicitly.

Attachments (1)

patch.diff (583 bytes) - added by taavi223 4 years ago.
Clarifies the documentation

Download all attachments as: .zip

Change History (7)

comment:1 Changed 4 years ago by julien

  • Component changed from Uncategorized to Documentation
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from TypedChoiceField does not coerce "" value, uses empty_value instead to Document that TypedChoiceField does not coerce empty_value
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

Coercing empty_value would be backwards-incompatible. Also, it is better practice and more pythonic for the developer to explicitly provide an already correctly-typed empty value if they wish to customise the coerce function. So it is a matter of clarifying the documentation to explain the current behaviour.

Changed 4 years ago by taavi223

Clarifies the documentation

comment:2 Changed 4 years ago by taavi223

  • Has patch set
  • Triage Stage changed from Accepted to Ready for checkin

I added a patch that clarifies the documentation. If there's anything else that needs to be added, please change the Triage Stage back to Accepted and let me know.

Last edited 4 years ago by taavi223 (previous) (diff)

comment:3 Changed 4 years ago by taavi223

  • Cc taavi223 added

comment:4 follow-up: Changed 4 years ago by julien

Thank you for your patch.

Just a note on the process. As mentioned in a few places in the contributing guide (for example, at the bottom of [1]), one should not promote their own patch to RFC and instead let someone else review the patch first. Then the ticket can be moved by that person to RFC otherwise comments can be left telling how the patch should be improved.

In this case, the patch looks good so I'm leaving the ticket in RFC. Don't hesitate to check out the contributing guide for more info on how to deal with future tickets. Cheers!

[1] https://docs.djangoproject.com/en/dev/internals/contributing/triaging-tickets/

comment:5 Changed 4 years ago by julien

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

In [16820]:

Fixed #16291 -- Documented that TypedChoiceField does not coerce empty_value. Thanks to vanschelven and taavi223.

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

Replying to julien:

Thank you for your patch.

Just a note on the process. As mentioned in a few places in the contributing guide (for example, at the bottom of [1]), one should not promote their own patch to RFC and instead let someone else review the patch first. Then the ticket can be moved by that person to RFC otherwise comments can be left telling how the patch should be improved.

In this case, the patch looks good so I'm leaving the ticket in RFC. Don't hesitate to check out the contributing guide for more info on how to deal with future tickets. Cheers!

[1] https://docs.djangoproject.com/en/dev/internals/contributing/triaging-tickets/

Whoops. Sorry about that. I looked through the docs the other day, but I was just skimming them and I somehow missed that. Maybe that's something I should add to my tutorial for first time contributors. :-)

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