[Django] #32810: django.utils.formats.number_format calculates a value for use_l10n which isn't re-used for calls to get_format

19 views
Skip to first unread message

Django

unread,
Jun 2, 2021, 7:38:10 AM6/2/21
to django-...@googlegroups.com
#32810: django.utils.formats.number_format calculates a value for use_l10n which
isn't re-used for calls to get_format
------------------------------------------------+------------------------
Reporter: Keryn Knight | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Utilities | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
Currently, the first thing `number_format` does is:

{{{
if use_l10n or (use_l10n is None and settings.USE_L10N):
lang = get_language()
else:
lang = None
}}}

so that it can pass the `lang` (string/None) and `use_l10n` (boolean/None)
through to the `get_format` function. The `get_format` function is used 3
times for the one `number_format` call (which in turn is used for every
call to `localize` via `render_value_in_context`)

The first thing `get_format` does, meanwhile, is much the same:
{{{
use_l10n = use_l10n or (use_l10n is None and settings.USE_L10N)
if use_l10n and lang is None:
lang = get_language()
}}}

As far as I can tell, a small micro-optimisation is available in
`number_format` to pre-calculate the `use_l10n` value and if it's truthy
avoid a few comparisons. I don't **think** it's subject to any threadlocal
values changing in between:

{{{
def number_format(value, decimal_pos=None, use_l10n=None,
force_grouping=False):
...
use_l10n = use_l10n or (use_l10n is None and settings.USE_L10N)
if use_l10n:
lang = get_language()
else:
lang = None
...
}}}
At worst it'd continue re-calculating the use_l10n value within
`get_format` because it's previously been evaluated as falsy, I think.

(Addendum: Having briefly glanced at it, I don't think this affects/steps
on the toes of #25762 which sounds like a much bigger/better win)

--
Ticket URL: <https://code.djangoproject.com/ticket/32810>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jun 3, 2021, 1:25:03 AM6/3/21
to django-...@googlegroups.com
#32810: django.utils.formats.number_format calculates a value for use_l10n which
isn't re-used for calls to get_format
--------------------------------------+------------------------------------

Reporter: Keryn Knight | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Mariusz Felisiak):

* easy: 0 => 1
* stage: Unreviewed => Accepted


--
Ticket URL: <https://code.djangoproject.com/ticket/32810#comment:1>

Django

unread,
Jun 3, 2021, 8:07:54 AM6/3/21
to django-...@googlegroups.com
#32810: django.utils.formats.number_format calculates a value for use_l10n which
isn't re-used for calls to get_format
-------------------------------------+-------------------------------------
Reporter: Keryn Knight | Owner: Mateo
Type: | Radman
Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mateo Radman):

* owner: nobody => Mateo Radman
* status: new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/32810#comment:2>

Django

unread,
Jun 5, 2021, 7:07:13 AM6/5/21
to django-...@googlegroups.com
#32810: django.utils.formats.number_format calculates a value for use_l10n which
isn't re-used for calls to get_format
-------------------------------------+-------------------------------------
Reporter: Keryn Knight | Owner: Mateo
Type: | Radman
Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mateo Radman):

* has_patch: 0 => 1


Comment:

Thanks Keryn, very good catch.
Fixed in https://github.com/django/django/pull/14492

--
Ticket URL: <https://code.djangoproject.com/ticket/32810#comment:3>

Django

unread,
Jun 5, 2021, 8:42:12 AM6/5/21
to django-...@googlegroups.com
#32810: django.utils.formats.number_format calculates a value for use_l10n which
isn't re-used for calls to get_format
-------------------------------------+-------------------------------------
Reporter: Keryn Knight | Owner: Mateo
Type: | Radman
Cleanup/optimization | Status: closed
Component: Utilities | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

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


Comment:

In [changeset:"d6f3b5858991e9609eaefc139dc605b36f9a07b3" d6f3b58]:
{{{
#!CommitTicketReference repository=""
revision="d6f3b5858991e9609eaefc139dc605b36f9a07b3"
Fixed #32810 -- Optimized django.utils.formats.number_format() a bit.

Pre-calculate use_l10n for get_format() calls.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/32810#comment:4>

Reply all
Reply to author
Forward
0 new messages