Wrap triangulate #731

weiji14 · 2020-12-14T12:39:10Z

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 triangulate for 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 using triangulate secretly at https://github.com/weiji14/pyissm/blob/5c9e3be4c4983ccc79a67cc6471421922e8c3af7/plot_figures.py#L68-L83 and thought it should get into pygmt proper 😄

The complicated thing about triangulate is that it generates either text (table) or NetCDF (grid) outputs, and I've implemented it similar to grdhisteq at #1433 as so:

triangulate.delaunay_triples:
- When output_type="numpy", output table as numpy.ndarray
- When output_type="pandas" (default for table), output table as pandas.DataFrame
- When output_type="file" and outfile is not None, output table as textfile
triangulate.regular_grid:
- When outgrid=True (default for grid), output grid as 'xarray.DataArray`
- When outgrid="test.nc", output grid to NetCDF file

Welcome to other suggestions on implementation details.

Fixes #

Reminders

Run make format and make check to make sure the code follows the style guide.
Add tests for new features or tests that would have caught the bug that you're fixing.
Add new public functions/methods/classes to doc/api/index.rst.
Write detailed docstrings for all functions/methods.
If adding new functionality, add an example to docstrings or tutorials.

Notes

You can write /format in the first line of a comment to lint the code automatically

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`.

seisman · 2020-12-15T07:25:27Z

When outgrid=None, output grid to NetCDF file

Typo?

When outgrid=None (default), output table as pandas.DataFrame
(Won't be implemented?), output table as textfile

I've never used this module, but how does GMT know the output type (a text file or a netcdf grid)?

weiji14 · 2020-12-15T08:20:14Z

When outgrid=None, output grid to NetCDF file

Typo?

Good catch, should be when outgrid="test.nc" I've updated top post.

When outgrid=None (default), output table as pandas.DataFrame
(Won't be implemented?), output table as textfile

I've never used this module, but how does GMT know the output type (a text file or a netcdf grid)?

So in pure GMT, triangulate outputs to a table format (technically stdout) with rows and columns. But using -Gtest.nc would interpolate that table result into a square grid output.

seisman · 2020-12-16T01:47:23Z

So in pure GMT, triangulate outputs to a table format (technically stdout) with rows and columns. But using -Gtest.nc would interpolate that table result into a square grid output.

OK. Two solutions for me:

Don't implement the "output to text file" feature. It's easy to save the returned pandas.DataFrame to a text file.
Add a new parameter outfile. Save the output to outfile if it's not None.

weiji14 · 2020-12-16T02:24:12Z

Don't implement the "output to text file" feature. It's easy to save the returned pandas.DataFrame to a text file.

Yep, Option 1 is the current state. Saving a pandas.DataFrame to an ASCII text file is definitely easier (compared to saving an xarray.DataArray to a NetCDF).

Add a new parameter outfile. Save the output to outfile if it's not None.

I thought about using outfile as well, and it's definitely implementable, though having 4 possible output types might confuse users. At some point though, this Option 2 might need to be done.

seisman · 2020-12-16T02:35:44Z

I think we can adopt option 1 now and add the outfile argument if requested.

Modernizing the code implementation to keep up to date with PyGMT v0.4.1. Also refreshing the unit tests a bit.

weiji14

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.

pygmt/src/triangulate.py

Co-authored-by: Will Schlitzer <[email protected]> Co-authored-by: Meghan Jones <[email protected]>

maxrjones

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.

pygmt/src/triangulate.py

Co-Authored-By: Meghan Jones <[email protected]>

weiji14 · 2022-03-13T19:42:30Z

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"
)

Yes an exception should be raised, but this is not specific to triangulate. I think we'll need to handle this across the board in #1318.

maxrjones · 2022-03-13T19:48:09Z

Yes an exception should be raised, but this is not specific to triangulate. I think we'll need to handle this across the board in #1318.

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

Looks good to me, thanks for all your work on this feature!

seisman

I've never used triangulate, so just two general comments.

pygmt/src/triangulate.py

Co-authored-by: Dongdong Tian <[email protected]>

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 <[email protected]> Co-authored-by: Meghan Jones <[email protected]> Co-authored-by: Dongdong Tian <[email protected]>

weiji14 added the feature Brand new feature label Dec 14, 2020

weiji14 force-pushed the master branch from 1730198 to 65b099a Compare February 15, 2021 12:06

weiji14 self-assigned this Mar 24, 2021

weiji14 mentioned this pull request May 24, 2021

Add common alias spacing (-I) for specifying grid increments #1288

Merged

31 tasks

weiji14 added this to the 0.5.0 milestone Jun 3, 2021

weiji14 added 2 commits August 11, 2021 11:33

Merge branch 'main' into gridding/triangulate

e56daee

Refactor triangulate to use virtualfile_from_data

3f4090e

Modernizing the code implementation to keep up to date with PyGMT v0.4.1. Also refreshing the unit tests a bit.

vercel bot temporarily deployed to Preview August 11, 2021 00:16 Inactive

weiji14 added 2 commits September 3, 2021 13:54

Merge branch 'main' into gridding/triangulate

9abefa6

Refactor triangulate implementation to use pygmt.io.load_dataarray

bd6bf57

vercel bot temporarily deployed to Preview September 3, 2021 01:59 Inactive

Alias binary(b), nodata(d), find(e), coltypes(f), header(h), incols(i)

cceb2a6

vercel bot temporarily deployed to Preview September 3, 2021 02:08 Inactive

weiji14 commented Sep 3, 2021

View reviewed changes

pygmt/src/triangulate.py Outdated Show resolved Hide resolved

willschlitzer reviewed Sep 9, 2021

View reviewed changes

pygmt/src/triangulate.py Outdated Show resolved Hide resolved

willschlitzer mentioned this pull request Sep 9, 2021

Fix grammatical error in triangulate docs GenericMappingTools/gmt#5741

Merged

4 tasks

maxrjones reviewed Sep 10, 2021

View reviewed changes

Apply suggestions from code review

cfb9572

Co-authored-by: Will Schlitzer <[email protected]> Co-authored-by: Meghan Jones <[email protected]>

vercel bot temporarily deployed to Preview September 10, 2021 21:32 Inactive

Merge branch 'main' into gridding/triangulate

c8c779b

vercel bot temporarily deployed to Preview September 10, 2021 21:42 Inactive

Wrap docstrings to 79 characters

9e67eef

vercel bot temporarily deployed to Preview September 11, 2021 04:53 Inactive

maxrjones mentioned this pull request Sep 11, 2021

Standardize parameter name for table data inputs #1479

Closed

Format fix

1f53beb

vercel bot temporarily deployed to Preview March 13, 2022 16:46 Inactive

maxrjones reviewed Mar 13, 2022

View reviewed changes

More triangulate docstring modifications

00d3b71

Co-Authored-By: Meghan Jones <[email protected]>

vercel bot temporarily deployed to Preview March 13, 2022 19:29 Inactive

weiji14 added 2 commits March 13, 2022 15:32

Wrap line to 79 characters

4ce402b

Actually document the output_type parameter for delaunay_triples

3006668

vercel bot temporarily deployed to Preview March 13, 2022 19:38 Inactive

maxrjones approved these changes Mar 13, 2022

View reviewed changes

maxrjones added the final review call This PR requires final review and approval from a second reviewer label Mar 13, 2022

maxrjones mentioned this pull request Mar 13, 2022

Changelog entry for v0.6.0 #1808

Merged

13 tasks

weiji14 mentioned this pull request Mar 14, 2022

None shall not pass args_in_kwargs #1815

Merged

6 tasks

seisman approved these changes Mar 14, 2022

View reviewed changes

pygmt/src/triangulate.py Outdated Show resolved Hide resolved

pygmt/src/triangulate.py Outdated Show resolved Hide resolved

Remove kwargs_to_strings decorator from methods

085620e

Co-authored-by: Dongdong Tian <[email protected]>

vercel bot temporarily deployed to Preview March 14, 2022 11:34 Inactive

Merge branch 'main' into gridding/triangulate

d221c96

vercel bot temporarily deployed to Preview March 14, 2022 11:55 Inactive

weiji14 removed the final review call This PR requires final review and approval from a second reviewer label Mar 14, 2022

weiji14 merged commit ba51f80 into main Mar 14, 2022

weiji14 deleted the gridding/triangulate branch March 14, 2022 12:15

weiji14 mentioned this pull request Apr 1, 2022

Refactor build_arg_string to also deal with infile and outfile #1837

Merged

6 tasks

weiji14 mentioned this pull request Nov 11, 2022

No aliases lists given in docs for some functions or methods #2185

Closed

weiji14 mentioned this pull request Oct 15, 2023

pygmt.which: Fix the bug when passing multiple files #2726

Merged

7 tasks

seisman mentioned this pull request Dec 18, 2023

Improve function signatures to use positional-only, positional-or-keyword, or keyword-only parameter #2893

Open

seisman mentioned this pull request Feb 27, 2024

pygmt.triangulate: Refactor to use codes consistent with other wrappers #3073

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrap triangulate #731

Wrap triangulate #731

weiji14 commented Dec 14, 2020 •

edited

Loading

seisman commented Dec 15, 2020

weiji14 commented Dec 15, 2020

seisman commented Dec 16, 2020

weiji14 commented Dec 16, 2020

seisman commented Dec 16, 2020

weiji14 left a comment

maxrjones left a comment

weiji14 commented Mar 13, 2022

maxrjones commented Mar 13, 2022

maxrjones left a comment

seisman left a comment

Wrap triangulate #731

Wrap triangulate #731

Conversation

weiji14 commented Dec 14, 2020 • edited Loading

seisman commented Dec 15, 2020

weiji14 commented Dec 15, 2020

seisman commented Dec 16, 2020

weiji14 commented Dec 16, 2020

seisman commented Dec 16, 2020

weiji14 left a comment

Choose a reason for hiding this comment

maxrjones left a comment

Choose a reason for hiding this comment

weiji14 commented Mar 13, 2022

maxrjones commented Mar 13, 2022

maxrjones left a comment

Choose a reason for hiding this comment

seisman left a comment

Choose a reason for hiding this comment

weiji14 commented Dec 14, 2020 •

edited

Loading