Opened 8 years ago

Last modified 7 years ago

#2537 new defect

Calculate doesn't accept "," as a decimal separator when set to Spanish locale

Reported by: garycmartin Owned by: rwh
Priority: Unspecified by Maintainer Milestone: Unspecified
Component: Calculate Version: Unspecified
Severity: Unspecified Keywords:
Cc: cjl Distribution/OS: Unspecified
Bug Status: Unconfirmed

Description

Tested on Calculate-34, when Sugar is set to a Spanish locale output values are correctly formatted using a "," as the decimal separator, however if you type an input value with a "," or click an output result to insert it into the input, the parser outputs an error.

Example: 10,5+4,5 should equal 15

Reported by Bernie via a dextrose only mail-list after receiving a bug report email from a Uruguayan math teacher.

Attachments (6)

0001-converting-from-locale-fraction_sep-before-parsing-2.patch (816 bytes) - added by walter 8 years ago.
replacing decimal_sep with '.'
0001-converting-from-locale-fraction_sep-before-parsing.patch (1.6 KB) - added by walter 8 years ago.
removed unnecessary import locale
0001-replace-fraction_sep-with-period-before-parsing-usin.patch (1.7 KB) - added by walter 8 years ago.
same thing only done with regex instead
0001-more-robust-handling-of-fraction-separator-regex-det.patch (1.9 KB) - added by walter 8 years ago.
0001-use-i18n-fraction_sep-on-decimal-button.patch (6.6 KB) - added by walter 8 years ago.
e.g., use ',' on decimal button in es_UY
0001-adding-i18n-support-for-fraction-separator.patch (2.7 KB) - added by walter 8 years ago.
combination of both patches

Download all attachments as: .zip

Change History (27)

comment:1 Changed 8 years ago by walter

Sound like there is a missing locale.atof() call

Changed 8 years ago by walter

replacing decimal_sep with '.'

comment:2 Changed 8 years ago by walter

The attached patch seems to work. It also seems a bit fragile, but I didn't want to delve into the black box that is ast.parser

comment:3 Changed 8 years ago by walter

This version of the patch handles more complex cases, such as plot(x*x,x=-1,0..1,0)

Note that there is a regression (unrelated to my patch) in v34 which I have not yet tracked down where using a rational number for the lower bound in a plot causes a parsing error. Stay tuned.

Changed 8 years ago by walter

removed unnecessary import locale

Changed 8 years ago by walter

same thing only done with regex instead

comment:4 Changed 8 years ago by walter

Attached is a version that is a bit more simple in that it uses regex. It also catches a few more cases, such as when a trailing decimal is omitted after the fraction separator. Please test.

comment:5 follow-up: Changed 8 years ago by walter

Note that the regression mentioned above seems to have been due to some error on my part... a fresh clone before applying my patch works fine.

comment:6 in reply to: ↑ 5 Changed 8 years ago by garycmartin

Replying to walter:

Note that the regression mentioned above seems to have been due to some error on my part... a fresh clone before applying my patch works fine.

Here are some cases that fail with the latest patch:

Input: plot(x^2,x=-2..2)
Output: plot(x^2.x=-2..2)
Error en 11: Error de procesamiento

Input: plot(x2,x=-2..2)
Output: plot(x2.x=-2..2)
Error en 11: Error de procesamiento

Input: add(10,5)
Output: add(10.5)
Error en 1: add() takes exactly 2 arguments (1 given)

Also typing 10,5+4,5 results in the correct answer of 15, but the output equation and history buffer is shown as 10.5+4.5

comment:7 follow-up: Changed 8 years ago by walter

Upon further investigation:

First, the statement at the top of the code is wrong: should be a lowercase utf

  • # -*- coding: UTF-8 -*-

+ # -*- coding: utf-8 -*-

Once that correction is made, then calculate works as is, without my patch.

But:

You need to be careful when typing in equations when comma is the fraction separator:

add(105) works
add(10,5) does not work

plot(x2x=-2..2) works

In other words, you need to make sure you disambiguate between commas as equation separators and commas as fraction separators and parse does everything else as expected.

As pointed out by Gary, the history buffer shows the parsed form of the equation, which uses a period as a separator. Not sure it is worth messing with at this point.

I'd recommend the correction to the language encoding line, documenting the comma-use convention, and then closing this ticket.

comment:8 in reply to: ↑ 7 Changed 8 years ago by garycmartin

Replying to walter:

First, the statement at the top of the code is wrong: should be a lowercase utf

  • # -*- coding: UTF-8 -*-

+ # -*- coding: utf-8 -*-

Once that correction is made, then calculate works as is, without my patch.

Hmmm, without any of your patches, and changing all the UTF statements to utf statements (there are four .py files with it in uppercase) I can't seen any discernible improvement in behaviour, e.g. with language set to Spanish and typing 10,5*2 fails like it did before with "Error: tipo no suportado".

Sorry if I've misread some step.

comment:9 Changed 8 years ago by walter

Hmmm. The only instance I changed was in calculate.py

I am running on an F14 system with Python 2.7. I'll test on some other systems.

comment:10 Changed 8 years ago by walter

Python 2.7 seems to behave differently than Python 2.6. My previous patch with one slight change will fix most of the problems, but I will add glue to detect which version Python is being used.

The change to the early patch is to change \d* to \d+, thus requiring a digit after the fraction-separator character in order to invoke the substitution.

Patch forthcoming.

comment:11 Changed 8 years ago by walter

Well, I was wrong about Python 2.7

The newly attached patch seems to do the trick in Gary's test cases, with the exception of the factorial example, which I think fails for unrelated reasons (factorial requires a int input).

Changed 8 years ago by walter

e.g., use ',' on decimal button in es_UY

comment:12 follow-up: Changed 8 years ago by walter

This somewhat convoluted patch will replace the '.' button with the locale's fraction separator.

In response to anacim's comment:

I can even mix the dot and the comma type 2.1 using the . button from the screen plus 3,5 using the comma from the keyboard and it works. It is somehow bizarre to see 2.1+3,5 on screen and obtain 5,6. I personally think that translating a calculator is not a very good idea..

comment:13 in reply to: ↑ 12 Changed 8 years ago by garycmartin

Replying to walter:

This somewhat convoluted patch will replace the '.' button with the locale's fraction separator.

Hmmm. Did you really intend to delete support for the new Sugar toolbars with your 0001-use-i18n-fraction_sep-on-decimal-button.patch?

In response to anacim's comment:

I can even mix the dot and the comma type 2.1 using the . button from the screen plus 3,5 using the comma from the keyboard and it works. It is somehow bizarre to see 2.1+3,5 on screen and obtain 5,6. I personally think that translating a calculator is not a very good idea..

comment:14 Changed 8 years ago by walter

Sorry about that. I must have applied the patch to an old version of the layout file. I'll pull a fresh copy and reapply my changes.

Changed 8 years ago by walter

combination of both patches

comment:15 follow-up: Changed 8 years ago by walter

http://bugs.sugarlabs.org/attachment/ticket/2537/0001-adding-i18n-support-for-fraction-separator.patch

One patch to address both problems and no longer accidentally reverting to old toolbar style.

comment:16 Changed 8 years ago by godiard

What is the state of this ticket?

Testing Calculate 37 I think is not resolved.

comment:17 Changed 8 years ago by walter

I don't think my patch ever made it in... doesn't seem to have been applied.

comment:18 Changed 8 years ago by garycmartin

I tried to land this in Calculate-37 but both times I applied and tested the patch (once on an older VM I use for development and once on a recent XO-1 candidate build) the is had no obvious effect (I could not use "," as a decimal separators in Spanish). I din't have time to debug and don't want to release with a patch I could not get to work.

comment:19 Changed 8 years ago by walter

I just reapplied it and it worked fine. Note that not every Spanish locale uses , as its fraction separator. Try using Spanish (Paraguay) for an example of a locale that uses a comma.

comment:20 in reply to: ↑ 15 Changed 7 years ago by garycmartin

Replying to walter:

http://bugs.sugarlabs.org/attachment/ticket/2537/0001-adding-i18n-support-for-fraction-separator.patch

One patch to address both problems and no longer accidentally reverting to old toolbar style.

OK, came very close to landing this but after retesting caught a nasty, common, case that is missed e.g. ,5*2 (where the zero before a decimal point is omitted).

comment:21 Changed 7 years ago by cjl

  • Cc cjl added
Note: See TracTickets for help on using tickets.