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.

Sign up for GitHub

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

Add libgdal-hdf5 dependency #293

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

Add libgdal-hdf5 dependency #293

Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ff8d995
Add libgdal-hdf5 dependency
seisman Jul 20, 2024
b5d9f04
Add libgdal-hdf5 and bump build number
seisman Jul 20, 2024
19de8d5
Fix the order
seisman Jul 20, 2024
e100863
Revert "Fix the order"
seisman Jul 20, 2024
f5add87
Revert "Add libgdal-hdf5 and bump build number"
seisman Jul 20, 2024
5587868
For using GDAL to read the file
seisman Jul 20, 2024
9a2f634
Add =gd to the input grid file
seisman Jul 20, 2024
290a214
Add libgdal-hdf5 and bump build number
seisman Jul 20, 2024
c38e0b8
Use a slight different region in the test
seisman Jul 20, 2024
f374cf3
Specify the GDAL driver
seisman Jul 20, 2024
9a33deb
Update recipe/meta.yaml
seisman Jul 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion recipe/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ source:
sha256: b17e165fd6c85aeb0a281700bd89522af8c2676a2d7bdb51a6b242fa9f1779c9

build:
number: 4
number: 5
skip: true # [ppc64le]

requirements:
Expand Down Expand Up @@ -43,6 +43,7 @@ requirements:
- geos
- ghostscript
- libgdal-jp2openjpeg
- libgdal-hdf5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sort h-i-j. Can you also add a test for reading a NetCDF/HDF5 file?

Suggested change
- libgdal-jp2openjpeg
- libgdal-hdf5
- libgdal-hdf5
- libgdal-jp2openjpeg

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add a test for reading a NetCDF/HDF5 file?

I added a test in ff8d995, and it worked without the libgdal-hdf5 library, but if I add libgdal-hdf5 in GenericMappingTools/pygmt@bb0efe3, tests passed (except one failure which is not directly related to the issue).

I think the reason is, when reading a netCDF file, GMT calls the netcdf library, but in GenericMappingTools/pygmt#3338, we're actually trying to read a netCDF file as an image (into a GMT_IMAGE container), which instead calls GDAL to read the file.

Copy link
Member

@weiji14 weiji14 Jul 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right... so should we actually add libgdal-hdf5 as a dependency here then? To be honest, that test_clib_read_data_image_actual_grid test in GenericMappingTools/pygmt#3338 is a bit weird since we're trying to read a 1-band grid as a GMT image.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, that test_clib_read_data_image_actual_grid test in GenericMappingTools/pygmt#3338 is a bit weird since we're trying to read a 1-band grid as a GMT image.

The original idea comes from GenericMappingTools/pygmt#3318 (comment) to solve the issue GenericMappingTools/pygmt#3115 (comment).

For any given file, we can call Session.read_data to read the file header only and determine the raster kind (grid, image or cube). It turns out, we can read ".nc" file as a GMT image and read ".tif" file as a GMT grid. So that test was added to understand what's exactly happening.

so should we actually add libgdal-hdf5 as a dependency here then?

We still need libgdal-hdf5, because reading other image formats via GDAL will likely need the HDF5 library.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or actually, can you add the test back and use the =gd modifier to force GMT to read the NetCDF file using GDAL? https://docs.generic-mapping-tools.org/6.5/reference/features.html#modifiers-to-read-and-write-grids-and-images-via-gdal

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

gmt grdcut @earth_relief_01d=gd -R-10/-9/3/5 -Greliefcut.nc

Test fails without libgdal-hdf5. See
https://github.com/conda-forge/gmt-feedstock/pull/293/checks?check_run_id=27695109707

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, I'll apply this to the dev branch for the next dev version update next week or so.

seisman marked this conversation as resolved.
Show resolved Hide resolved
- libnetcdf
- hdf5
- {{ pin_compatible('zlib', max_pin='x.x') }}
Expand Down
Loading