Skip to content

Update tile retreival ahn.py#371

Merged
bdestombe merged 3 commits intodevfrom
bdestombe-ahn-patch-1
Sep 24, 2024
Merged

Update tile retreival ahn.py#371
bdestombe merged 3 commits intodevfrom
bdestombe-ahn-patch-1

Conversation

@bdestombe
Copy link
Collaborator

  • On the eclipse server: Skip tile retreival if the link does not exist.
  • Legacy server: Rasterio merge returned only zero's since 17 sept. All the merging is now done within xarray.

- On the eclipse server: Skip tile retreival if the link does not exist.
- Legacy server: Rasterio merge returned only zero's since 17 sept. All the merging is now done within xarray.
@rubencalje
Copy link
Collaborator

Great.

I would just leave the old method (_download_and_combine_tiles) intact, and not change anything. With the proposed changes, and when as_data_array is False, you return something else as before . This method is just for people who wish to use the exact same output as before, using the legacy-methods. So changing its output-type is kind of contradictory to this. In a few versions, we will remove the legacy-methods and _download_and_combine_tiles.

@bdestombe
Copy link
Collaborator Author

Well could you maybe confirm? If I use _download_and_combine_tiles(), during the merge command of rasterio all values are converted to 0. With the proposed changes the function works like intended.

@bdestombe
Copy link
Collaborator Author

Since no error is produced and zeros are falsely returned, I would suggest to either

  • already remove these functions,
  • keep these functions as legacy functions with an additional warning that their outcome is wrong
  • keep these suggested changes

@rubencalje
Copy link
Collaborator

The zeros are caused by rasterio version 1.3.11, and you do get a warning (see issue #369). We changed the normal versions of the methods to use the xarray-merge to solve this bug. However, some people might want to use the old server, or as_data_array=False. Therefore, the old methods are still available using the _legacy-suffix. However, I do not think we should maintain these methods. This will introduce additional work and errors.

For example. With the proposed changes, the following code

import nlmod
extent = [100_000, 110_000, 400_000, 405_000]
nlmod.read.ahn.get_ahn4_legacy(extent)

produces

<xarray.DataArray 'band_data' (band: 1, y: 0, x: 2000)> Size: 0B
array([], shape=(1, 0, 2000), dtype=float32)
Coordinates:
  * band         (band) int32 4B 1
  * x            (x) float64 16kB 1e+05 1e+05 1e+05 ... 1.1e+05 1.1e+05 1.1e+05
  * y            (y) float64 0B 
    spatial_ref  int32 4B 0
Attributes:
    AREA_OR_POINT:  Area

So this is an empty data-array (while the original methods produce the correct data-array, with non-zero data if you do not use rasterio 1.3.11). We can off course fix this issue as well, but this is not the goal of the legacy-methods.

@bdestombe
Copy link
Collaborator Author

bdestombe commented Sep 23, 2024

Thanks Ruben for taking the time. I was falsely under the impression that the all-zeros-bug also appeared with other versions of rasterio. I will revert the code and merge the two remaining lines.

Edit: I was mixing up rioxarray and rasterio packages when checking the behavior for different versions.. With rasterio version 1.3.11 and changing versions of rioxarray the all-zeros-bug of course remains

@bdestombe bdestombe merged commit dc85c48 into dev Sep 24, 2024
@bdestombe bdestombe deleted the bdestombe-ahn-patch-1 branch September 24, 2024 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants