[Django] #25762: Optimize numberformat.format

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #25762: Optimize numberformat.format

Django
#25762: Optimize numberformat.format
--------------------------------------+--------------------
     Reporter:  jaap3                 |      Owner:  nobody
         Type:  Cleanup/optimization  |     Status:  new
    Component:  Utilities             |    Version:  master
     Severity:  Normal                |   Keywords:
 Triage Stage:  Unreviewed            |  Has patch:  1
Easy pickings:  0                     |      UI/UX:  0
--------------------------------------+--------------------
 `numberformat.format` is used a lot by Django and in most cases it's
 called with a number. Yet the current implementation is pretty much built
 for strings.

 I've refactored the function to be up to 4x faster on my machine (and in
 the worst case it performs roughly the same). Attached is the benchmark
 script I've used.

--
Ticket URL: <https://code.djangoproject.com/ticket/25762>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/048.bee5643a83373a6602b24f94622e220e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #25762: Optimize numberformat.format

Django
#25762: Optimize numberformat.format
----------------------------------+----------------------------
 Reporter:  jaap3                 |          Owner:  nobody
     Type:  Cleanup/optimization  |         Status:  new
Component:  Utilities             |        Version:  master
 Severity:  Normal                |     Resolution:
 Keywords:                        |   Triage Stage:  Unreviewed
Has patch:  1                     |  Easy pickings:  0
    UI/UX:  0                     |
----------------------------------+----------------------------
Changes (by jaap3):

 * Attachment "bench_numberformat.py" added.

 Benchmark for numberformat.format

--
Ticket URL: <https://code.djangoproject.com/ticket/25762>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/063.f69d3a6f2b18c6b1e53e08cb6b309c5d%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #25762: Optimize numberformat.format

Django
In reply to this post by Django
#25762: Optimize numberformat.format
--------------------------------------+------------------------------------
     Reporter:  jaap3                 |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     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 timgraham):

 * needs_better_patch:   => 0
 * needs_docs:   => 0
 * needs_tests:   => 0
 * stage:  Unreviewed => Accepted


Comment:

 I'll mention [https://github.com/django/djangobench/ djangobench] in case
 you want to adapt any of your testing scripts so they can live on.

--
Ticket URL: <https://code.djangoproject.com/ticket/25762#comment:1>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/063.19499c037978c42f509ad2e46b73d35e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #25762: Optimize numberformat.format

Django
In reply to this post by Django
#25762: Optimize numberformat.format
--------------------------------------+------------------------------------
     Reporter:  jaap3                 |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     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
--------------------------------------+------------------------------------

Comment (by jaap3):

 Ah nice, I'll look into adding some benchmarks to djangobench

--
Ticket URL: <https://code.djangoproject.com/ticket/25762#comment:2>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/063.fea4abe3b2985068acc209a4267de8ec%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #25762: Optimize numberformat.format

Django
In reply to this post by Django
#25762: Optimize numberformat.format
--------------------------------------+------------------------------------
     Reporter:  jaap3                 |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     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
--------------------------------------+------------------------------------

Comment (by jaap3):

 By the way, while refactoring this code I was wondering if supporting
 NUMBER_GROUPING other than 3 is even useful. None of the locales shipped
 with Django define anything other than 3, and the other related
 configuration options (THOUSAND_SEPARATOR and USE_THOUSAND_SEPARATOR) all
 refer to grouping by 3 anyway.

 It would also be nice if number formatting on strings was deprecated. It
 seems that Django already tries to call it with numbers in the places it's
 used, and there's no real check if the strings that are passed into the
 function are even number like.

--
Ticket URL: <https://code.djangoproject.com/ticket/25762#comment:3>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/063.8db5e75bd136408a842af375e6e62320%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #25762: Optimize numberformat.format

Django
In reply to this post by Django
#25762: Optimize numberformat.format
--------------------------------------+------------------------------------
     Reporter:  jaap3                 |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------
Changes (by timgraham):

 * needs_better_patch:  0 => 1


Comment:

 Left comments for improvement on the pull request. Maybe you'd like to
 raise the other questions on the DevelopersMailingList. I'm not sure about
 the answers myself.

--
Ticket URL: <https://code.djangoproject.com/ticket/25762#comment:4>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/063.fa7fa325562c5a458edc9338983fb6ca%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #25762: Optimize numberformat.format

Django
In reply to this post by Django
#25762: Optimize numberformat.format
-------------------------------------+-------------------------------------
     Reporter:  Jaap Roes            |                    Owner:  tim-
         Type:                       |  mccurrach
  Cleanup/optimization               |                   Status:  assigned
    Component:  Utilities            |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by tim-mccurrach):

 * status:  new => assigned
 * owner:  nobody => tim-mccurrach


--
Ticket URL: <https://code.djangoproject.com/ticket/25762#comment:5>
Django <https://code.djangoproject.com/>
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/063.9e03e9d050d67578bfeecd1cd85fd25f%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #25762: Optimize numberformat.format

Django
In reply to this post by Django
#25762: Optimize numberformat.format
-------------------------------------+-------------------------------------
     Reporter:  Jaap Roes            |                    Owner:  tim-
         Type:                       |  mccurrach
  Cleanup/optimization               |                   Status:  assigned
    Component:  Utilities            |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by tim-mccurrach):

 The approach used in the linked PR
 (https://github.com/django/django/pull/5668/files) actually has a problem.
 The following approach is used to avoid the rounding behaviour of the
 builtin `format` function:
 {{{
                 # If `number` is 10.888 and `decimal_pos` is 2 return
 10.88 not 10.89.
                 format_string = '.%sf' % (decimal_pos + 1)
 }}}
 and the string is then truncated to `decimal_pos` decimals places.
 However, for a number such as 10.999, this will still be rounded 11.00.
 This is inconsistent with the behaviour of the rest of the function, which
 truncates and doesn't round.

 We therefore can't use the bultin `format` to do any kind of decimal
 formatting as there doesn't appear (as it currently stands) to be an
 option to truncate with it, however the speed advantages it offers can
 still be used for the common case of seperating a number into thousands.

 Using the test cases in `bench_number_format.py` linked above, an improved
 function performs anywhere between 2x and 5x faster for most cases, and
 again in the worst case is roughly the same (fluctuating between +2% and
 -2% compared with the original function).

 I'll add a new PR.

--
Ticket URL: <https://code.djangoproject.com/ticket/25762#comment:6>
Django <https://code.djangoproject.com/>
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/063.eda003801fef1f4c3138e5f516d27263%40djangoproject.com.