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

qgswmsgetcapabilities: Use float format for min/max scale parameters #60058

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

ptitjano
Copy link
Contributor

@ptitjano ptitjano commented Jan 6, 2025

Description

MinScaleDenominator and MaxScaleDenominator string are created from a QString::number call with the default parameters. In that case, the most concise format is used. This can result in the scientific notation being used for very big or small numbers.

With this change, these parameters are now always displayed in a float format. Reasons for this change:

  • this might prevent a loss of precision for very small or big numbers
  • this output is not supposed to be human readable so it does not matter if is not concise
  • this is the logic used by a lot of WMS servers, for example arcgis server

cc @troopa81

@github-actions github-actions bot added this to the 3.42.0 milestone Jan 6, 2025
Copy link

github-actions bot commented Jan 6, 2025

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 183d1ce)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 183d1ce)

@ptitjano ptitjano closed this Jan 6, 2025
@ptitjano ptitjano reopened this Jan 6, 2025
@rouault
Copy link
Contributor

rouault commented Jan 7, 2025

Can we have a test that checks the truncation works ?

@Gustry Gustry added the Server Related to QGIS server label Jan 7, 2025
@ptitjano
Copy link
Contributor Author

ptitjano commented Jan 7, 2025

Can we have a test that checks the truncation works ?

Indeed! To be honest, I was convinced that there was already a unit test but I was unable to find it. I was "hoping" that one of the tests would fail! I will add one.

@ptitjano ptitjano force-pushed the wms-scale-denominator-format branch from 1c44022 to 7a3b757 Compare January 7, 2025 19:52
@ptitjano
Copy link
Contributor Author

ptitjano commented Jan 7, 2025

Can we have a test that checks the truncation works ?

I have added a unit test.

@ptitjano ptitjano self-assigned this Jan 7, 2025
tests/src/python/test_qgsserver_wms.py Outdated Show resolved Hide resolved
tests/src/python/test_qgsserver_wms.py Outdated Show resolved Hide resolved
`MinScaleDenominator` and `MaxScaleDenominator` string are created
from a `QString::number` call with the default parameters. In that
case, the most concise format is used. This can result in the
scientific notation being used for very big or small numbers.

With this change, these parameters are now always displayed in a float
format. Reasons for this change:
- this might prevent a loss of precision for very small or big numbers
- this output is not supposed to be human readable so it does not
 matter if is not concise
- this is the logic used by a lot of WMS servers, for example arcgis
 server
@ptitjano ptitjano force-pushed the wms-scale-denominator-format branch from 7a3b757 to 183d1ce Compare January 8, 2025 16:47
@elpaso elpaso merged commit 0984ab8 into qgis:master Jan 9, 2025
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release-3_40 Server Related to QGIS server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants