geopython / OWSLib

OWSLib is a Python package for client programming with Open Geospatial Consortium (OGC) web service (hence OWS) interface standards, and their related content models.
https://owslib.readthedocs.io
BSD 3-Clause "New" or "Revised" License
392 stars 278 forks source link

FIX: numpy floats parsed incorrectly #955

Closed juhi24 closed 2 weeks ago

juhi24 commented 2 weeks ago

Referring to https://github.com/SciTools/cartopy/issues/2472#issuecomment-2457518215

Currently owslib cannot handle bbox defined as numpy.float64. This PR fixes the referred issue by using str instead of repr.

geographika commented 2 weeks ago

Thanks @juhi24. The CI looks like it is failing on unrelated issues.

It looks like this line was originally request['bbox'] = ','.join([str(x) for x in bbox]) but was updated to use repr in https://github.com/geopython/OWSLib/commit/c9e2ddb426683f8b0036e370826906a8030241c5 - the reason being "changing str to repr to maintain precision in bbox coords".

It would be good to add a test with a high precision bbox values to check if str strips values off the end.

Also linking to a related issue #878.

juhi24 commented 2 weeks ago

Hi, @geographika. I cannot reproduce the difference in precision in the earliest and latest supported Python versions 3.10 or 3.13, nor in legacy 3.6. All give:

>>> f = 123.4567890123456789
>>> str(f)
'123.45678901234568'
>>> repr(f)
'123.45678901234568'

I think this was a Python 2 issue:

>>> f = 123.4567890123456789
>>> str(f)
'123.456789012'
>>> repr(f)
'123.45678901234568'

The https://github.com/geopython/OWSLib/commit/c9e2ddb426683f8b0036e370826906a8030241c5 was committed in 2011.

geographika commented 2 weeks ago

@juhi24 - thanks for checking this, I'm +1 on merging.

It would be good to add in a test just confirming the precision for a bbox is retained. I think adding a simple unit test for __build_getmap_request (for WMS 1.3.0) to test_wms_getmap.py would work, without needing to make an actual request to a server. If you're able to add this to the pull request that would be great.

juhi24 commented 2 weeks ago

@geographika, test added and passing.

geographika commented 2 weeks ago

Just for future reference a float is truncated when converting with str, but in Python3 it is rounded to the same number of decimal places as repr so functionality will be unchanged with this pull request. To have higher accuracy use decimals - this approach would not have been possible using repr.

>>> f = 123.4567890123456789
>>> str(f)
'123.45678901234568'
>>> repr(f)
'123.45678901234568'

# to have higher precision use decimals - str converts correctly
>>> str(d)
'123.4567890123456805895330035127699375152587890625'
>>> repr(d)
"Decimal('123.4567890123456805895330035127699375152587890625')"
# this would fail to create a bbox

Thanks for the test and fix @juhi24.