Opened 3 years ago
Closed 3 years ago
#32810 closed Cleanup/optimization (fixed)
django.utils.formats.number_format calculates a value for use_l10n which isn't re-used for calls to get_format
Reported by: | Keryn Knight | Owned by: | Mateo Radman |
---|---|---|---|
Component: | Utilities | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
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)
Change History (4)
comment:1 by , 3 years ago
Easy pickings: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 3 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 3 years ago
Has patch: | set |
---|
Thanks Keryn, very good catch.
Fixed in https://github.com/django/django/pull/14492