Re: [Django] #36801: `construct_change_message`call `form.changed_data` after check if is add

2026-01-16 Thread Django 
#36801: `construct_change_message`call `form.changed_data` after check if is add
-+-
 Reporter:  Rodolfo Becerra  |Owner:  Rodolfo
 Type:   |  Becerra
  Cleanup/optimization   |   Status:  closed
Component:  contrib.admin|  Version:
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
  construct_change_message   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-
Changes (by Jacob Walls ):

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

Comment:

 In [changeset:"a77e541d1d8766b0e7d2c4143076ef07973f5bd9" a77e541]:
 {{{#!CommitTicketReference repository=""
 revision="a77e541d1d8766b0e7d2c4143076ef07973f5bd9"
 Fixed #36801 -- Avoided unnecessary calculation in
 construct_change_message().

 `changed_field_labels` is only needed if there are changes to log, so move
 its
 calculation, including the somewhat costly `translation_override()`,
 inside the
 conditional that checks for changes. Also avoid reading
 `form.changed_data`
 when it’s already bound to `changed_data`.

 co-authored-by: Rodolfo Becerra
 <[email protected]>
 }}}
-- 
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 [email protected].
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/0107019bc72b96a3-1f5e5678-1bc2-4bb3-bbb6-c3b731b2f039-00%40eu-central-1.amazonses.com.

Re: [Django] #36801: `construct_change_message`call `form.changed_data` after check if is add

2026-01-12 Thread Django 
#36801: `construct_change_message`call `form.changed_data` after check if is add
-+-
 Reporter:  Rodolfo Becerra  |Owner:  Rodolfo
 Type:   |  Becerra
  Cleanup/optimization   |   Status:  new
Component:  contrib.admin|  Version:
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Ready for
  construct_change_message   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-
Changes (by Jacob Walls):

 * stage:  Unreviewed => Ready for checkin

Comment:

 Thanks, that helps.
-- 
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 [email protected].
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/0107019bb38afd23-65dd322c-c39b-48ef-b158-be4e39cbeeba-00%40eu-central-1.amazonses.com.

Re: [Django] #36801: `construct_change_message`call `form.changed_data` after check if is add

2026-01-10 Thread Django 
#36801: `construct_change_message`call `form.changed_data` after check if is add
-+-
 Reporter:  Rodolfo Becerra  |Owner:  Rodolfo
 Type:   |  Becerra
  Cleanup/optimization   |   Status:  new
Component:  contrib.admin|  Version:
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
  construct_change_message   |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-
Changes (by Adam Johnson):

 * has_patch:  0 => 1
 * resolution:  needsinfo =>
 * status:  closed => new

Comment:

 Rodolfo created this ticket at my encouragement, after he showed me a diff
 of the change he meant. I think the description didn't quite do it
 justice, so I've created a PR to show what he meant. I think it's a pretty
 clear optimization.
-- 
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 [email protected].
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/0107019ba9a96a1f-69da47e1-bd1c-4387-a07c-31175a77a242-00%40eu-central-1.amazonses.com.

Re: [Django] #36801: `construct_change_message`call `form.changed_data` after check if is add

2025-12-15 Thread Django 
#36801: `construct_change_message`call `form.changed_data` after check if is add
-+-
 Reporter:  Rodolfo Becerra  |Owner:  Rodolfo
 Type:   |  Becerra
  Cleanup/optimization   |   Status:  closed
Component:  contrib.admin|  Version:
 Severity:  Normal   |   Resolution:  needsinfo
 Keywords:   | Triage Stage:
  construct_change_message   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-
Changes (by Jacob Walls):

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

Comment:

 Hi Rodolfo. So `changed_data` is a cached property to avoid having to
 reason about this in various places. Given that, I'm inclined to close as
 `needsinfo` pending a benchmark showing this is worth devoting a review
 cycle to. This is generally how we handle all optimization proposals.
-- 
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 [email protected].
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/0107019b248de68c-42312150-9db1-4ff9-a46b-36dcd7cdcb75-00%40eu-central-1.amazonses.com.

Re: [Django] #36801: `construct_change_message`call `form.changed_data` after check if is add

2025-12-15 Thread Django 
#36801: `construct_change_message`call `form.changed_data` after check if is add
-+-
 Reporter:  Rodolfo Becerra  |Owner:  Rodolfo
 Type:   |  Becerra
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.admin|  Version:
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
  construct_change_message   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-
Description changed by Rodolfo Becerra:

Old description:

> This is the currently code of the method `construct_change_message` of
> `django.contrib.admin.utils`
> {{{
> #!python
> def construct_change_message(form, formsets, add):
> """
> Construct a JSON structure describing changes from a changed object.
> Translations are deactivated so that strings are stored untranslated.
> Translation happens later on LogEntry access.
> """
> # Evaluating `form.changed_data` prior to disabling translations is
> # required to avoid fields affected by localization from being
> included
> # incorrectly, e.g. where date formats differ such as MM/DD/ vs
> # DD/MM/.
> changed_data = form.changed_data
> with translation_override(None):
> # Deactivate translations while fetching verbose_name for form
> # field labels and using `field_name`, if verbose_name is not
> provided.
> # Translations will happen later on LogEntry access.
> changed_field_labels = _get_changed_field_labels_from_form(form,
> changed_data)
>
> change_message = []
> if add:
> change_message.append({"added": {}})
> elif form.changed_data:
> change_message.append({"changed": {"fields":
> changed_field_labels}})
> }}}
> If you notice that the `form.changed_data` property is being called
> unnecessarily before checking whether it is an add behavior. I propose
> call the `changed_data = form.changed_data` after check if is add.

New description:

 This is the currently code of the method `construct_change_message` of
 `django.contrib.admin.utils`
 {{{
 #!python
 def construct_change_message(form, formsets, add):
 """
 Construct a JSON structure describing changes from a changed object.
 Translations are deactivated so that strings are stored untranslated.
 Translation happens later on LogEntry access.
 """
 # Evaluating `form.changed_data` prior to disabling translations is
 # required to avoid fields affected by localization from being
 included
 # incorrectly, e.g. where date formats differ such as MM/DD/ vs
 # DD/MM/.
 changed_data = form.changed_data
 with translation_override(None):
 # Deactivate translations while fetching verbose_name for form
 # field labels and using `field_name`, if verbose_name is not
 provided.
 # Translations will happen later on LogEntry access.
 changed_field_labels = _get_changed_field_labels_from_form(form,
 changed_data)

 change_message = []
 if add:
 change_message.append({"added": {}})
 elif form.changed_data:
 change_message.append({"changed": {"fields":
 changed_field_labels}})
 }}}
 If you notice that the `form.changed_data` property is being called
 unnecessarily before checking whether it is an add behavior. I propose
 call the `changed_data = form.changed_data` after check if is add.

 If I am accepted, I would like to create the PR myself.

--
-- 
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 [email protected].
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/0107019b22a1640a-4f742801-e154-4043-829d-ec0ded2db564-00%40eu-central-1.amazonses.com.

[Django] #36801: `construct_change_message`call `form.changed_data` after check if is add

2025-12-15 Thread Django 
#36801: `construct_change_message`call `form.changed_data` after check if is add
-+-
   Reporter:  Rodolfo|  Owner:  Rodolfo Becerra
  Becerra|
   Type: | Status:  assigned
  Cleanup/optimization   |
  Component: |Version:
  contrib.admin  |   Keywords:
   Severity:  Normal |  construct_change_message
   Triage Stage: |  Has patch:  0
  Unreviewed |
Needs documentation:  0  |Needs tests:  0
Patch needs improvement:  0  |  Easy pickings:  1
  UI/UX:  0  |
-+-
 This is the currently code of the method `construct_change_message` of
 `django.contrib.admin.utils`
 {{{
 #!python
 def construct_change_message(form, formsets, add):
 """
 Construct a JSON structure describing changes from a changed object.
 Translations are deactivated so that strings are stored untranslated.
 Translation happens later on LogEntry access.
 """
 # Evaluating `form.changed_data` prior to disabling translations is
 # required to avoid fields affected by localization from being
 included
 # incorrectly, e.g. where date formats differ such as MM/DD/ vs
 # DD/MM/.
 changed_data = form.changed_data
 with translation_override(None):
 # Deactivate translations while fetching verbose_name for form
 # field labels and using `field_name`, if verbose_name is not
 provided.
 # Translations will happen later on LogEntry access.
 changed_field_labels = _get_changed_field_labels_from_form(form,
 changed_data)

 change_message = []
 if add:
 change_message.append({"added": {}})
 elif form.changed_data:
 change_message.append({"changed": {"fields":
 changed_field_labels}})
 }}}
 If you notice that the `form.changed_data` property is being called
 unnecessarily before checking whether it is an add behavior. I propose
 call the `changed_data = form.changed_data` after check if is add.
-- 
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 [email protected].
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/0107019b22a06815-7bb98e68-0dc6-404e-a0fb-9bff16f7c90c-00%40eu-central-1.amazonses.com.