Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
pygmt.binstats: Add alias "tiling" for "T" #2409
base: main
Are you sure you want to change the base?
pygmt.binstats: Add alias "tiling" for "T" #2409
Changes from 7 commits
354d2c4
b8c0f9b
ed41595
230f39c
0d9088d
c692fb7
5bff1cf
41c2cfd
a237093
a52d8c2
991e39a
9ed9c90
68f25d9
6a604f9
d328199
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
tiling="h"
outputs the statistics to a table, which was why this alias was not added in the orginal implementation, see #1652 (comment).The way we've handle functions that output to either a grid or table was discussed in #1536, which is to use a "Two methods in a single Python class" like
pygmt.triangulate
andpygmt.grdhisteq
. Unfortunately, if we follow this style, that would require breaking changes topygmt.binstats
🙂There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, so @maxrjones mentioned at #1652 (review) that the flag to output statistics to a table is actually more similar to #896 (which is for
fig.*
plotting functions)? Butgmtbinstats
is not a plotting function, so this is kinda awkward 😅There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @weiji14 for this clarification and background information. As
tiling
is already used in the docstring forsearch_radius
I thought that adding the doctstring fortiling
was simply overlooked...I am unsure how to continue with this PR. Should we leave it open or should we have a separate issue for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's leave the PR open for now, the discussion on whether to turn
pygmt.binstats
into two separate methods (e.g.pygmt.binstats.to_grid
orpygmt.binstats.to_table
) can happen here. Anything more general can also be discussed on the open thread at #1536.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand this means that it's also not possible to plot the hexagons outlines (polygons )that are used to calculate the stats but to get the center coordinates and stats values for the corresponding hexagon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michaelgrund what is your concrete use-case here? Please note, that hexagonal binning is only available for Cartesian data and that only the y-increment can be given and GMT calculates the x-increment given the geometry (https://docs.generic-mapping-tools.org/latest/gmtbinstats.html#t). Based on the upstream GMT documentation, I feel it is not possible to get the outline of the hexagons directly.
Are you looking for figures like this:
In some cases, one can try to determine the (regular) hexagon based on the center of the hexagon (first two columns of the output file via
outgrid
) and the spacing between the centers in y-direction (value passed tospacing
). The outline can then be plotted (be careful with width and height of the figure) using the centers of the hexagon andstyle="h" + str(determined_diameter_of_hexagon) + "c"
. Additionally, the stats output can be used to color-code the fill of the hexagons (please un-comment the corresponding parts in the code below). However, I feel there are other libraries / packages which are more suitable to generate such a visualization.Code to reproduce the figures shown above:
Test data (Please place it in your work dictionary): test_data_in.txt