Opened 7 years ago

Closed 6 years ago

#6783 closed (fixed)

DecimalField tests with locale

Reported by: Dirk Datzert <dummy@…> Owned by: garcia_marc
Component: Internationalization Version: master
Severity: Keywords: i18n-fixed
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description (last modified by ramiro)

I ran into an python locale issue with the DecimalField. During INSERTs and UPDATEs invalid sql-statements are generated since a comma-seperator ',' is used for formating DecimalField instead of the expected dot-seperator '.'

decimal-python.py (for simple testing):

from django.db.models.fields import DecimalField
from locale import setlocale, LC_NUMERIC
d = DecimalField(max_digits=6, decimal_places=3)
setlocale(LC_NUMERIC,'de_DE')
print d.format_number(3.456)
setlocale(LC_NUMERIC,'en_US')
print d.format_number(3.456)

linux-f426d:/home/admin/django/projects # export DJANGO_SETTINGS_MODULE=ais.settings
linux-f426d:/home/admin/django/projects # python decimal-python.py              
3,456
3.456

Environment:

  • Django-SVN
  • SuSE Linux Enterprise Server 10
  • Python 2.4.2

On my private notebook this is no issue both outputs will be

3.456
3.456

Private Environment:

  • Django-SVN
  • OpenSuSe 10.3
  • Python 2.5.1

Attachments (7)

django-DecimalField-prep_locale.patch (1.5 KB) - added by Dirk Datzert <dummy@…> 7 years ago.
short work-around for DecimalField format problems in db_prep methods
django-DecimalField-prep_locale.2.patch (1.5 KB) - added by Dirk Datzert <dummy@…> 7 years ago.
short work-around for DecimalField format problems in db_prep methods
6783_tests.diff (1.0 KB) - added by PhiR 7 years ago.
base tests for the issue
6783_tests.2.diff (1.6 KB) - added by PhiR 7 years ago.
slightly better tests
simple_patch.diff (451 bytes) - added by yazzgoth 7 years ago.
6793_tests-svn-11368.diff (1.6 KB) - added by Dirk Datzert <dummy@…> 6 years ago.
Tests against Django 1.1 trunk rev 11368
6783-tests-logging.txt (36.6 KB) - added by Dirk Datzert <dummy@…> 6 years ago.
Verbose Log output on running model_regress test which fails

Download all attachments as: .zip

Change History (18)

Changed 7 years ago by Dirk Datzert <dummy@…>

short work-around for DecimalField format problems in db_prep methods

Changed 7 years ago by Dirk Datzert <dummy@…>

short work-around for DecimalField format problems in db_prep methods

Changed 7 years ago by PhiR

base tests for the issue

comment:1 Changed 7 years ago by PhiR

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

my test machine doesn't have the appropriate locales but I'll run the tests tomorrow.

Changed 7 years ago by PhiR

slightly better tests

comment:2 Changed 7 years ago by PhiR

  • Summary changed from DecimalField python locale issue to DecimalField tests with locale
  • Triage Stage changed from Unreviewed to Ready for checkin

The tests all pass with sqlite and mysql. I tried with both python 2.4.4 and 2.5.1. I'm marking ready for checking so we can commit the regression tests and hopefully get those run with other backends/versions.

Please note that the tests need the 'fr_FR' locale to run. If it's not available the test will be omitted (no failure though). If anyone has a more elegant solution I'd be happy to update the patch :)

comment:3 Changed 7 years ago by PhiR

  • Owner changed from nobody to PhiR

comment:4 Changed 7 years ago by mtredinnick

  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

Two problems here:

  1. On a triage level, the test changes should have been part of the main patch. It took a while to realise that this "ready for checkin" thing was actually the second and the fourth patch.
  2. More importantly, the solution isn't correct. setlocale() cannot be used in multi-threaded programs, since it changes the global locale, so it will affect the operation of other threads.

Changed 7 years ago by yazzgoth

comment:5 Changed 7 years ago by yazzgoth

I'm sorry to hijack this ticket.

I've uploaded a simple_patch.diff that should is a quick and dirty way of sorting this out.

comment:6 Changed 7 years ago by ramiro

  • Description modified (diff)

comment:7 Changed 7 years ago by PhiR

  • Owner PhiR deleted

mtredinnick: sorry I wasn't explicit, but actually my plan was to commit only the tests, not the OP's patch. I haven't been able to reproduce the issue even though I use FR_fr on my servers.

comment:8 Changed 6 years ago by garcia_marc

  • Keywords i18n-fixed added
  • Owner set to garcia_marc

Changed 6 years ago by Dirk Datzert <dummy@…>

Tests against Django 1.1 trunk rev 11368

Changed 6 years ago by Dirk Datzert <dummy@…>

Verbose Log output on running model_regress test which fails

comment:9 Changed 6 years ago by garcia_marc

  • Triage Stage changed from Accepted to Unreviewed

I understand what's failing, but I don't understand what are you trying to achieve by using setlocale. Do you use it for displaying data on templates, and then you get errors from the db?

If that's the case, we should close this ticket as invalid, so the problem is not in Django, but in your code. As Malcolm says, you can't use setlocale in Django. Of course another ticket about being able to change number formats on Django would be correct. Actually it's not necessary, so this is already possible on the i18n branch.

comment:10 Changed 6 years ago by jacob

  • Triage Stage changed from Unreviewed to Accepted

comment:11 Changed 6 years ago by jezdez

  • Resolution set to fixed
  • Status changed from new to closed

(In [11964]) Fixed #7980 - Improved i18n framework to support locale aware formatting (dates and numbers) and form processing.

Thanks to Marc Garcia for working on this during his Google Summer of Code 2009!

Additionally fixes #1061, #2203, #3940, #5526, #6449, #6231, #6693, #6783, #9366 and #10891.

Note: See TracTickets for help on using tickets.
Back to Top