search

GenericMappingTools / pygmt

A Python interface for the Generic Mapping Tools.
https://www.pygmt.org
BSD 3-Clause "New" or "Revised" License
758 stars 220 forks source link

Optional 'drape_input' in pygmt.Figure.grdview #1467

Open NikosAlexandris opened 3 years ago

NikosAlexandris commented 3 years ago

An idea: add a drape_input= option pygmt.Figure.grdview() -- this relates to the discussions in https://github.com/GenericMappingTools/gmt/issues/5631. Perhaps something like drape_red=, drape_green= and drape_blue= which should eventually be mutually exclusive with drape_input=.

A general comment: GMT is super-nice! Yet one thing I find hard is its non-declarative names of options and flags and else. In my humble view, fully spelling out names of options, parametes and flags and else, is best towards human-readable code.

welcome[bot] commented 3 years ago

👋 Thanks for opening your first issue here! Please make sure you filled out the template with as much detail as possible. You might also want to take a look at our contributing guidelines and code of conduct.

weiji14 commented 3 years ago

Hi @NikosAlexandris, really sorry for the late reply! Just a note that PyGMT's fig.grdview does have a drapegrid parameter which should be the same as the drape_input option you mentioned (specifically, -G in GMT grdview), made available since #330 (PyGMT v0.1.0)! This drapegrid= parameter should support both xarray.DataArray and file inputs (though file inputs might be more stable from experience).

If your :sparkles: feature request is related to supporting drape_red=, drape_green= and drape_blue= in PyGMT, then well, we'll need to wait for the outcome of the discussion at https://github.com/GenericMappingTools/gmt/issues/5631. Specifically, whether passing in separate RGB bands to drapegrid/-G is future-proof. Ping @meghanrjones who might know more.

In the meantime, I'm working on grdmix at #1437 which would be able to combine Red/Green/Blue grids (e.g. GeoTiffs) into a single image that could then be passed into grdview's drapegrid= parameter (due for PyGMT v0.5.0). If you can describe your specific use-case (e.g. what satellite products are you using? are you combining GeoTiffs directly, or an xarray.Dataset?), that will be very helpful for us to inform the code implementation details.

A general comment: GMT is super-nice! Yet one thing I find hard is its non-declarative names of options and flags and else. In my humble view, fully spelling out names of options, parametes and flags and else, is best towards human-readable code.

Yes, GMT is the product of ~30 years of work! Most of the functionality is there already, and I guess it's up to us to make it usable for the masses :slightly_smiling_face:

maxrjones commented 3 years ago

If your ✨ feature request is related to supporting drape_red=, drape_green= and drape_blue= in PyGMT, then well, we'll need to wait for the outcome of the discussion at GenericMappingTools/gmt#5631. Specifically, whether passing in separate RGB bands to drapegrid/-G is future-proof. Ping @meghanrjones who might know more.

I think the plan is to move to just grid or image input for grdview -G rather than red, green, and blue bands. That said, depending on your viewpoint passing in separate RGB bands to drapegrid/-G could be considered future proof because while GMT may issue 'deprecations', all these old options and syntax are still supported under the hood 🙃

My opinion is that it would be worth adding this feature to PyGMT if someone has time to implement it regardless of the plan for https://github.com/GenericMappingTools/gmt/issues/5631. I do not think that https://github.com/GenericMappingTools/gmt/issues/5631 will be resolved quickly, passing individual bands can be easier than making the region of the image perfect in order to work with grdview, and we could still support passing drape_red=, drape_green, and drape_blue= in PyGMT using an internal call to pygmt.grdmix (once finalized) independent of the plan for grdview -G.