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

Locale support for extent and raster save as dlg #41316

Merged

Conversation

elpaso
Copy link
Contributor

@elpaso elpaso commented Feb 2, 2021

Fixes #41284

@elpaso elpaso added Bug Either a bug report, or a bug fix. Let's hope for the latter! backport release-3_16 labels Feb 2, 2021
@elpaso
Copy link
Contributor Author

elpaso commented Feb 2, 2021

CC @SrNetoChan , @agiudiceandrea @gioman

Can you please check this out? Also test nodata and pyramids (no locale support on the latter)

@github-actions github-actions bot added this to the 3.18.0 milestone Feb 2, 2021
@agiudiceandrea
Copy link
Contributor

Hi @elpaso, the behavior now is not completely fixed.

On Windows, with an Italian Locale it_IT (comma as decimal separator), it is now possible to enter both a comma or a point in the extent settings or in the Columns and Rows settings, but if a point is used it is not interpreted as a decimal separator but silently discarded: e.g. 100.5 is interpreted as 1005.

I think that QgsDoubleValidator::toDouble should be used, instead of QLocale().toDouble(), to convert form the QLineEdit text to double value in order to correctly handle the number validated by QgsDoubleValidator.

@agiudiceandrea
Copy link
Contributor

agiudiceandrea commented Feb 2, 2021

@elpaso, I don't know if it is related to this PR, but QGIS (mxe artifact build from this PR) crashes every time a layer is removed from the map if there is only one layer in the map.

Co-authored-by: Andrea Giudiceandrea <andreaerdna@libero.it>
@elpaso
Copy link
Contributor Author

elpaso commented Feb 2, 2021

@elpaso, I don't know if it is related to this PR, but QGIS (mxe artifact build from this PR) crashes every time a layer is removed from the map if there is only one layer in the map.

Probably unrelated.

Thank you for the review btw!

@agiudiceandrea
Copy link
Contributor

Probably unrelated.

You are right, The offending PR is #41285 (comment). A patch was submitted right now #41319.

@SrNetoChan
Copy link
Member

@elpaso tested your PR on Ubuntu 18.04

Everything works as expected.

There were also problems in the nodata options. If you used commas to set a nodata interval from 0 to 100,5 , the nodata option would be ignored and the raster was saved without setting those values to NODATA.

In your PR it seems to be working.

From this experience, I would say we should/could search for more QString::number and text().toDouble() in gui elements as they are probably causing problems somehow, no?

@elpaso
Copy link
Contributor Author

elpaso commented Feb 3, 2021

@elpaso tested your PR on Ubuntu 18.04

Everything works as expected.

Thanks for checking!

There were also problems in the nodata options. If you used commas to set a nodata interval from 0 to 100,5 , the nodata option would be ignored and the raster was saved without setting those values to NODATA.

In your PR it seems to be working.

From this experience, I would say we should/could search for more QString::number and text().toDouble() in gui elements as they are probably causing problems somehow, no?

Definitely.

@elpaso
Copy link
Contributor Author

elpaso commented Feb 3, 2021

unrelated test failure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

raster export (save as...) not working if QGIS locale uses commas as decimal separator
3 participants