Opened 16 years ago

Closed 14 years ago

#6783 closed (fixed)

DecimalField tests with locale

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

Description (last modified by Ramiro Morales)

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

Download all attachments as: .zip

Change History (18)

by Dirk Datzert <dummy@…>, 16 years ago

short work-around for DecimalField format problems in db_prep methods

by Dirk Datzert <dummy@…>, 16 years ago

short work-around for DecimalField format problems in db_prep methods

by Philippe Raoult, 16 years ago

Attachment: 6783_tests.diff added

base tests for the issue

comment:1 by Philippe Raoult, 16 years ago

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

by Philippe Raoult, 16 years ago

Attachment: 6783_tests.2.diff added

slightly better tests

comment:2 by Philippe Raoult, 16 years ago

Summary: DecimalField python locale issueDecimalField tests with locale
Triage Stage: UnreviewedReady 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 by Philippe Raoult, 16 years ago

Owner: changed from nobody to Philippe Raoult

comment:4 by Malcolm Tredinnick, 16 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

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.

by yazzgoth, 16 years ago

Attachment: simple_patch.diff added

comment:5 by yazzgoth, 16 years ago

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 by Ramiro Morales, 16 years ago

Description: modified (diff)

comment:7 by Philippe Raoult, 15 years ago

Owner: Philippe Raoult removed

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 by Marc Garcia, 15 years ago

Keywords: i18n-fixed added
Owner: set to Marc Garcia

by Dirk Datzert <dummy@…>, 15 years ago

Attachment: 6793_tests-svn-11368.diff added

Tests against Django 1.1 trunk rev 11368

by Dirk Datzert <dummy@…>, 15 years ago

Attachment: 6783-tests-logging.txt added

Verbose Log output on running model_regress test which fails

comment:9 by Marc Garcia, 15 years ago

Triage Stage: AcceptedUnreviewed

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 by Jacob, 14 years ago

Triage Stage: UnreviewedAccepted

comment:11 by Jannis Leidel, 14 years ago

Resolution: fixed
Status: newclosed

(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