Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Localization issue when entering decimals in mrview on Windows #447

Closed
bjeurissen opened this issue Feb 22, 2016 · 10 comments
Closed

Localization issue when entering decimals in mrview on Windows #447

bjeurissen opened this issue Feb 22, 2016 · 10 comments

Comments

@bjeurissen
Copy link
Member

On Windows, my localization is set to Dutch (Belgium), which mean the decimal separator is a comma instead of a point. This causes the text boxes in mrview to completely ignore decimal separators, regardless of whether I use a comma or a point.

Changing the decimal separator in the windows localization settings to point fixes the issue.

This is not an issue on Mac OS X, where I can just use the point separator even though my localization is set to comma.

@bjeurissen bjeurissen changed the title Localization issue when entering floating point numbers in mrview on Windows Localization issue when entering decimals in mrview on Windows Feb 22, 2016
@jdtournier
Copy link
Member

I assume this a problem specifically with the text entries that can be adjusted with the mouse...? These are instances of the AdjustButton class. If that's the case, I have a feeling this will be due to a conflict between the QDoubleValidator, used internally by our AdjustButton class, and our to<float>() and str() calls. The former will presumably respect your locale, whereas the MRtrix3 calls explicitly won't (this is because they're used to read & write values in the image headers, etc, and you can't have the data stored in a locale-specific manner). But these are all used within the AdjustButton class...

There's two options here: fix the AdjustButton class to honour the locale (so avoid using the to<float>() and str() calls), or fix it to ignore the locale. I'm not sure which approach would be best. The problem is any Qt class that isn't AdjustButton will probably honour the locale, whereas everythingb else in MRtrix3 won't - e.g. the floating-point values displayed in the main window (position and intensity) will use a decimal point regardless. The proper thing would probably be to respect the locale, but personally I would prefer not to - we've already encountered issues with that, I have a feeling it would be cleaner to avoid such issues occurring in the first place. I'm thinking of edge cases like copy-pasting values from the viewer into a file, and getting weird results using this file because the bulk of the code ignores the locale. Not a common use case, I'll concede, but the point is I think it's cleaner to stick to a single format throughout...

@bjeurissen
Copy link
Member Author

I would be happy with mrview ignoring the locale. Keeps things simple and
avoids similar issues popping up.
On 23 Feb 2016 6:38 p.m., "J-Donald Tournier" notifications@github.com
wrote:

I assume this a problem specifically with the text entries that can be
adjusted with the mouse...? These are instances of the AdjustButton class.
If that's the case, I have a feeling this will be due to a conflict between
the QDoubleValidator, used internally by our AdjustButton class, and our
to() and str() calls. The former will presumably respect your
locale, whereas the MRtrix3 calls explicitly won't (this is because they're
used to read & write values in the image headers, etc, and you can't have
the data stored in a locale-specific manner). But these are all used within
the AdjustButton class...

There's two options here: fix the AdjustButton class to honour the locale
(so avoid using the to() and str() calls), or fix it to ignore the
locale. I'm not sure which approach would be best. The problem is any Qt
class that isn't AdjustButton will probably honour the locale, whereas
everythingb else in MRtrix3 won't - e.g. the floating-point values
displayed in the main window (position and intensity) will use a decimal
point regardless. The proper thing would probably be to respect the locale,
but personally I would prefer not to - we've already encountered issues
with that, I have a feeling it would be cleaner to avoid such issues
occurring in the first place. I'm thinking of edge cases like copy-pasting
values from the viewer into a file, and getting weird results using this
file because the bulk of the code ignores the locale. Not a common use
case, I'll concede, but the point is I think it's cleaner to stick to a
single format throughou t...


Reply to this email directly or view it on GitHub
#447 (comment).

@jdtournier
Copy link
Member

Cool. In which case, we need to implement our own validator - or figure out if we can override the locale assumed by the existing QValidator. By the way, when I say 'we', I mean someone with more time on their hands than me... 😉

@jdtournier
Copy link
Member

On that note: it might be that we can override the locale at the global application level, which would probably be the best thing to do anyway...

@bjeurissen
Copy link
Member Author

http://doc.qt.io/qt-5/qlocale.html#setDefault might do the trick.

@jdtournier
Copy link
Member

Seems perfect. Guess the best place to put it would be here? Do you want to give this a go?

@bjeurissen
Copy link
Member Author

Will do. Do you think QLocale::setDefault(QLocale::c()) will do, or do we need a "fancier" locale?

@bjeurissen
Copy link
Member Author

Seems to fix the problem with the decimal separator.

@jdtournier
Copy link
Member

I'm happy with just the standard C locale... Go for it.

@bjeurissen
Copy link
Member Author

Guess this is closed...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants