Opened 13 years ago
Last modified 11 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)
Change History (27)
comment:1 Changed 13 years ago by walter
comment:2 Changed 13 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 13 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.
comment:4 Changed 13 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: ↓ 6 Changed 13 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 13 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: ↓ 8 Changed 13 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 13 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 13 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 13 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 13 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 13 years ago by walter
comment:12 follow-up: ↓ 13 Changed 13 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 13 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 13 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.
comment:15 follow-up: ↓ 20 Changed 13 years ago by walter
One patch to address both problems and no longer accidentally reverting to old toolbar style.
comment:16 Changed 12 years ago by godiard
What is the state of this ticket?
Testing Calculate 37 I think is not resolved.
comment:17 Changed 12 years ago by walter
I don't think my patch ever made it in... doesn't seem to have been applied.
comment:18 Changed 12 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 12 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 12 years ago by garycmartin
Replying to walter:
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 11 years ago by cjl
- Cc cjl added
Sound like there is a missing locale.atof() call