Re: [Django] #32920: BaseForm's _clean_fields() and changed_data should access values via BoundField

[Django]

#32920: BaseForm's _clean_fields() and changed_data should access values via
BoundField
-+-
 Reporter:  Chris Jerdonek   |Owner:  Chris
 Type:   |  Jerdonek
  Cleanup/optimization   |   Status:  closed
Component:  Forms|  Version:  dev
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Carlton Gibson ):

 In [changeset:"08f077888548a951f01b454d0db08d9407f7f0aa" 08f07788]:
 {{{
 #!CommitTicketReference repository=""
 revision="08f077888548a951f01b454d0db08d9407f7f0aa"
 Refs #32920 -- Added BoundField._has_changed() for use in
 BaseForm.changed_data().
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.51f40c2251e507078779a83d3af04e6f%40djangoproject.com.

Re: [Django] #32920: BaseForm's _clean_fields() and changed_data should access values via BoundField

[Django]

#32920: BaseForm's _clean_fields() and changed_data should access values via
BoundField
-+-
 Reporter:  Chris Jerdonek   |Owner:  Chris
 Type:   |  Jerdonek
  Cleanup/optimization   |   Status:  closed
Component:  Forms|  Version:  dev
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Carlton Gibson ):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"90a33ab2ceddef7f2cdd11612f77ea9296cc7fb9" 90a33ab2]:
 {{{
 #!CommitTicketReference repository=""
 revision="90a33ab2ceddef7f2cdd11612f77ea9296cc7fb9"
 Fixed #32920 -- Changed BaseForm to access its values through bound
 fields.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.b12f94e47763c466058bfc1966d40f88%40djangoproject.com.

Re: [Django] #32920: BaseForm's _clean_fields() and changed_data should access values via BoundField

[Django]

#32920: BaseForm's _clean_fields() and changed_data should access values via
BoundField
-+-
 Reporter:  Chris Jerdonek   |Owner:  Chris
 Type:   |  Jerdonek
  Cleanup/optimization   |   Status:  assigned
Component:  Forms|  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Carlton Gibson):

 * stage:  Accepted => Ready for checkin


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.d6c197962bb7f10545c85d8d5d95e007%40djangoproject.com.

Re: [Django] #32920: BaseForm's _clean_fields() and changed_data should access values via BoundField

[Django]

#32920: BaseForm's _clean_fields() and changed_data should access values via
BoundField
-+-
 Reporter:  Chris Jerdonek   |Owner:  Chris
 Type:   |  Jerdonek
  Cleanup/optimization   |   Status:  assigned
Component:  Forms|  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Chris Jerdonek):

 * has_patch:  0 => 1


Comment:

 PR: https://github.com/django/django/pull/14631

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.a36ce8e59c012d600d38b1f11a6b7e5d%40djangoproject.com.

Re: [Django] #32920: BaseForm's _clean_fields() and changed_data should access values via BoundField

[Django]

#32920: BaseForm's _clean_fields() and changed_data should access values via
BoundField
-+-
 Reporter:  Chris Jerdonek   |Owner:  Chris
 Type:   |  Jerdonek
  Cleanup/optimization   |   Status:  assigned
Component:  Forms|  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Carlton Gibson):

 * stage:  Unreviewed => Accepted


Comment:

 Hey Chris, this is interesting, and makes sense. :) — I think we should
 have a look at this yes. Thanks!

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.fb65229c9483c4d994eef7ebce803f3e%40djangoproject.com.

Re: [Django] #32920: BaseForm's _clean_fields() and changed_data should access values via BoundField

[Django]

#32920: BaseForm's _clean_fields() and changed_data should access values via
BoundField
-+-
 Reporter:  Chris Jerdonek   |Owner:  Chris
 Type:   |  Jerdonek
  Cleanup/optimization   |   Status:  assigned
Component:  Forms|  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Description changed by Chris Jerdonek:

Old description:

> While working on #32917, I noticed that
> [https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/django/forms/forms.py#L386-L388
> BaseForm._clean_fields()] and
> [https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/django/forms/forms.py#L436
> BaseForm.changed_data] don't currently access their values through a
> `BoundField` object. It would be better for consistency if they did, and
> to reduce the number of code paths.
>
> One consequence of the current code is that `form._clean_fields()` can
> return a different value from `form[name].initial` when they should be
> the same. This case is almost, but not quite, covered by
> [https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/tests/forms_tests/tests/test_forms.py#L2115-L2123
> test_datetime_clean_initial_callable_disabled()] (the test can be
> adjusted to cover this case).
>
> As part of this ticket and in line with accessing data through the
> `BoundField` objects, I noticed that the code would also be simpler if
> the per-field logic of `changed_data()` were moved into a method of the
> `BoundField` class. It could be called something like `bf.did_change()`.
> This would be more appropriate because whether form data changed for a
> field is a property of its `BoundField` (as it depends on the underlying
> form data), as opposed to the unbound field. With this change, the method
> could change from its current ~20 lines to something like this--
>
> {{{#!python
> @cached_property
> def changed_data(self):
> return [name for name, bf in self._bound_items() if bf._did_change()]
> }}}

New description:

 While working on #32917, I noticed that
 
[https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/django/forms/forms.py#L386-L388
 BaseForm._clean_fields()] and
 
[https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/django/forms/forms.py#L436
 BaseForm.changed_data] don't currently access their values through a
 `BoundField` object. It would be better for consistency if they did, and
 to reduce the number of code paths.

 One consequence of the current code is that `form._clean_fields()` can
 return a different value from `form[name].initial` when they should be the
 same. This case is almost, but not quite, covered by
 
[https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/tests/forms_tests/tests/test_forms.py#L2115-L2123
 test_datetime_clean_initial_callable_disabled()] (the test can be adjusted
 to cover this case).

 As part of this ticket and in line with accessing data through the
 `BoundField` objects, I noticed that the code would also be simpler if the
 per-field logic of `changed_data()` were moved into a method of the
 `BoundField` class. It could be called something like `bf.did_change()`.
 This would be more appropriate because whether form data changed for a
 field is a property of its `BoundField` (as it depends on the underlying
 form data), as opposed to the unbound field. With this change, the method
 could change from its current ~20 lines to something like this--

 {{{#!python
 @cached_property
 def changed_data(self):
 return [name for name, bf in self._bound_items() if bf._did_change()]
 }}}

 A similar change could be made to `BaseForm._clean_fields()`.

--

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.0a8a454c779584de2db4bd08e254c437%40djangoproject.com.

Re: [Django] #32920: BaseForm's _clean_fields() and changed_data should access values via BoundField

[Django]

#32920: BaseForm's _clean_fields() and changed_data should access values via
BoundField
-+-
 Reporter:  Chris Jerdonek   |Owner:  Chris
 Type:   |  Jerdonek
  Cleanup/optimization   |   Status:  assigned
Component:  Forms|  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Description changed by Chris Jerdonek:

Old description:

> While working on #32917, I noticed that
> [https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/django/forms/forms.py#L386-L388
> BaseForm._clean_fields()] and
> [https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/django/forms/forms.py#L436
> BaseForm.changed_data] don't currently access their values through a
> `BoundField` object. It would be better for consistency if they did, and
> to reduce the number of code paths.
>
> One consequence of the current code is that `form._clean_fields()` can
> return a different value from `form[name].initial` when they should be
> the same. This case is almost, but not quite, covered by
> [https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/tests/forms_tests/tests/test_forms.py#L2115-L2123
> test_datetime_clean_initial_callable_disabled()] (the test can be
> adjusted to cover this case).
>
> As part of this ticket and in line with accessing data through the
> `BoundField` objects, I noticed that the code would also be simpler if
> the per-field logic of `changed_data()` were moved into a method of the
> `BoundField` class. It could be called something like `bf.did_change()`.
> This would be more appropriate because whether form data changed for a
> field is a property of its `BoundField` (as it depends on the underlying
> form data), as opposed to the unbound field.

New description:

 While working on #32917, I noticed that
 
[https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/django/forms/forms.py#L386-L388
 BaseForm._clean_fields()] and
 
[https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/django/forms/forms.py#L436
 BaseForm.changed_data] don't currently access their values through a
 `BoundField` object. It would be better for consistency if they did, and
 to reduce the number of code paths.

 One consequence of the current code is that `form._clean_fields()` can
 return a different value from `form[name].initial` when they should be the
 same. This case is almost, but not quite, covered by
 
[https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/tests/forms_tests/tests/test_forms.py#L2115-L2123
 test_datetime_clean_initial_callable_disabled()] (the test can be adjusted
 to cover this case).

 As part of this ticket and in line with accessing data through the
 `BoundField` objects, I noticed that the code would also be simpler if the
 per-field logic of `changed_data()` were moved into a method of the
 `BoundField` class. It could be called something like `bf.did_change()`.
 This would be more appropriate because whether form data changed for a
 field is a property of its `BoundField` (as it depends on the underlying
 form data), as opposed to the unbound field. With this change, the method
 could change from its current ~20 lines to something like this--

 {{{#!python
 @cached_property
 def changed_data(self):
 return [name for name, bf in self._bound_items() if bf._did_change()]
 }}}

--

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.e33dfc807228d4ec065dc373b0a9b311%40djangoproject.com.

Re: [Django] #32920: BaseForm's _clean_fields() and changed_data should access values via BoundField

[Django]

#32920: BaseForm's _clean_fields() and changed_data should access values via
BoundField
-+-
 Reporter:  Chris Jerdonek   |Owner:  Chris
 Type:   |  Jerdonek
  Cleanup/optimization   |   Status:  assigned
Component:  Forms|  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Chris Jerdonek):

 Here is how to make the failing test I mentioned above (roughly):

 {{{
  def test_datetime_clean_initial_callable_disabled(self):
 -now = datetime.datetime(2006, 10, 25, 14, 30, 45, 123456)
 -
  class DateTimeForm(forms.Form):
 -dt = DateTimeField(initial=lambda: now, disabled=True)
 +dt = DateTimeField(initial=datetime.datetime.now,
 disabled=True)

  form = DateTimeForm({})
  self.assertEqual(form.errors, {})
 -self.assertEqual(form.cleaned_data, {'dt': now})
 +cleaned_value = form.cleaned_data['dt']
 +bf = form['dt']
 +self.assertEqual(cleaned_value, bf.initial)
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.b3c31fdc94f3a48eec597b4aae8e1c6c%40djangoproject.com.

[Django] #32920: BaseForm's _clean_fields() and changed_data should access values via BoundField

[Django]

#32920: BaseForm's _clean_fields() and changed_data should access values via
BoundField
-+-
   Reporter:  Chris  |  Owner:  Chris Jerdonek
  Jerdonek   |
   Type: | Status:  assigned
  Cleanup/optimization   |
  Component:  Forms  |Version:  dev
   Severity:  Normal |   Keywords:
   Triage Stage: |  Has patch:  0
  Unreviewed |
Needs documentation:  0  |Needs tests:  0
Patch needs improvement:  0  |  Easy pickings:  0
  UI/UX:  0  |
-+-
 While working on #32917, I noticed that
 
[https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/django/forms/forms.py#L386-L388
 BaseForm._clean_fields()] and
 
[https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/django/forms/forms.py#L436
 BaseForm.changed_data] don't currently access their values through a
 `BoundField` object. It would be better for consistency if they did, and
 to reduce the number of code paths.

 One consequence of the current code is that `form._clean_fields()` can
 return a different value from `form[name].initial` when they should be the
 same. This case is almost, but not quite, covered by
 
[https://github.com/django/django/blob/0250340e372f652c4f276e6874d452d683c94dfe/tests/forms_tests/tests/test_forms.py#L2115-L2123
 test_datetime_clean_initial_callable_disabled()] (the test can be adjusted
 to cover this case).

 As part of this ticket and in line with accessing data through the
 `BoundField` objects, I noticed that the code would also be simpler if the
 per-field logic of `changed_data()` were moved into a method of the
 `BoundField` class. It could be called something like `bf.did_change()`.
 This would be more appropriate because whether form data changed for a
 field is a property of its `BoundField` (as it depends on the underlying
 form data), as opposed to the unbound field.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/052.2bbfb2f976795d78ba321f3b5ff38b6a%40djangoproject.com.