Conversation
Wrapping the triangulate function which does "Delaunay triangulation or Voronoi partitioning and gridding of Cartesian data" under gridding.py. Original GMT documentation can be found at https://docs.generic-mapping-tools.org/6.1/triangulate.html. Aliased outgrid (G), spacing (I), projection (J), region (R), verbose (V), registration (r). Also included 8 unit tests modified from `surface`.
Typo?
I've never used this module, but how does GMT know the output type (a text file or a netcdf grid)? |
Good catch, should be when
So in pure GMT, |
OK. Two solutions for me:
|
Yep, Option 1 is the current state. Saving a
I thought about using |
|
I think we can adopt option 1 now and add the |
Modernizing the code implementation to keep up to date with PyGMT v0.4.1. Also refreshing the unit tests a bit.
weiji14
left a comment
There was a problem hiding this comment.
This is a bit of an old PR (~9 months), but a good one to put in for PyGMT v0.5.0 I think. Ping @GenericMappingTools/pygmt-maintainers for a review if free. The triangulate function as implemented takes an input table, and outputs either a table or grid.
Co-authored-by: Will Schlitzer <schlitzer90@gmail.com> Co-authored-by: Meghan Jones <meghanj@alum.mit.edu>
Co-authored-by: Meghan Jones <meghanj@alum.mit.edu>
maxrjones
left a comment
There was a problem hiding this comment.
Great work! These should be my last comments.
Should an exception be raised if output_type == "file" and outfile is None? For example:
pygmt.triangulate.delaunay_triples(
data="@Table_5_11_mean.xyz",
output_type="file"
)
The ongoing issues with uninformative error messages affect this PR, but that shouldn't hold back this feature.
Co-Authored-By: Meghan Jones <meghanrjones@users.noreply.github.com>
Yes an exception should be raised, but this is not specific to |
Sounds good to deal with this separately (after v0.6.0). Of course I would have liked to fix that issue before this release, but this year has been going by too fast... |
maxrjones
left a comment
There was a problem hiding this comment.
Looks good to me, thanks for all your work on this feature!
seisman
left a comment
There was a problem hiding this comment.
I've never used triangulate, so just two general comments.
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
Wrapping the triangulate function which does "Delaunay triangulation or Voronoi partitioning and gridding of Cartesian data". Original GMT documentation can be found at https://docs.generic-mapping-tools.org/6.3/triangulate.html. Aliased outgrid (G), spacing (I), projection (J), region (R), verbose (V), registration (r). * Refactor triangulate to use virtualfile_from_data * Refactor triangulate implementation to use pygmt.io.load_dataarray * Alias binary(b), nodata(d), find(e), coltypes(f), header(h), incols(i) * Rename the parameter 'table' to 'data' As per GenericMappingTools#1479. * Refactor test_triangulate to use Table_5_11_mean.xyz instead of tut_ship * Refactor test_triangulate_with_outgrid to use xr.testing.assert_allclose * Refactor test_triangulate_input_xyz to use pd.testing.assert_frame_equal * Implement regular_grid and delaunay_triples staticmethod for triangulate * Let list inputs to spacing (I) and incols (i) work Use I="sequence" and i="sequence_comma". * Ensure triangulate.delaunay_triples output_type is valid Must be either one of numpy, pandas or file * Autocorrect output_type to 'file' if outfile parameter is set * Allow only str or None inputs to outgrid parameter Xref GenericMappingTools#1807 * Use gmt get GMT_TRIANGULATE to check whether Watson or Shewchuk is used * State that Shewchuk is the default triangulation algorithm As per GenericMappingTools/gmt#6438 * Actually document the output_type parameter for delaunay_triples Co-authored-by: Will Schlitzer <schlitzer90@gmail.com> Co-authored-by: Meghan Jones <meghanj@alum.mit.edu> Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
Description of proposed changes
Wrapping the triangulate function which does "Delaunay triangulation or Voronoi partitioning and gridding of Cartesian data", implemented under gridding.py. For a GMT example, see https://docs.generic-mapping-tools.org/6.3/gallery/ex16.html
Preview for this Pull Request is at https://pygmt-git-gridding-triangulate-gmt.vercel.app/api/generated/pygmt.triangulate.html
Note that this is a fairly minimal implementation of
triangulatefor now, and I don't expect to wrap and test all the aliases here as they are super complicated. Just that I've just been usingtriangulatesecretly at https://github.com/weiji14/pyissm/blob/5c9e3be4c4983ccc79a67cc6471421922e8c3af7/plot_figures.py#L68-L83 and thought it should get intopygmtproper 😄The complicated thing about
triangulateis that it generates either text (table) or NetCDF (grid) outputs, and I've implemented it similar togrdhisteqat #1433 as so:triangulate.delaunay_triples:output_type="numpy", output table asnumpy.ndarrayoutput_type="pandas"(default for table), output table aspandas.DataFrameoutput_type="file"andoutfile is not None, output table as textfiletriangulate.regular_grid:outgrid=True(default for grid), output grid as 'xarray.DataArray`outgrid="test.nc", output grid to NetCDF fileWelcome to other suggestions on implementation details.
Fixes #
Reminders
make formatandmake checkto make sure the code follows the style guide.doc/api/index.rst.Notes
/formatin the first line of a comment to lint the code automatically