GMT_GRID_HEADER: Parse grid header and add grid properties by seisman · Pull Request #3134 · GenericMappingTools/pygmt

seisman · 2024-03-22T11:39:48Z

Description of proposed changes

Part of PR #2398.

This PR add codes to parse the grid header and get the dimension names, attributes, gtype and more. They're stored as properties of the header.

The codes can be reused in wrapping GMT_GRID/GMT_IMAGE/GMT_CUBE data structures.

Tests are added to the _parse_nameunits function but not for other functions. The function will be tested in the GMT_GRID.to_dataarray method.

… and units from x/y/z_units

Co-authored-by: Michael Grund <23025878+michaelgrund@users.noreply.github.com>

Co-authored-by: Yvonne Fröhlich <94163266+yvonnefroehlich@users.noreply.github.com>

weiji14 · 2024-03-29T03:01:05Z

+ # Name of data set
+ ("title", ctp.c_char * GMT_GRID_TITLE_LEN80),
+ # Name of generating command
+ ("command", ctp.c_char * GMT_GRID_COMMAND_LEN320),
+ # Comments for this data set
+ ("remark", ctp.c_char * GMT_GRID_REMARK_LEN160),


Store these metadata in the attrs dict too?

Done in dfbc2e5.

Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>

seisman · 2024-03-29T04:17:11Z

I'm wondering if we should move the _parse_nameunits/_parse_header functions as the GMT_GRID_HEADER class methods, so that we don't have to import these private functions when wrapping GMT_GRID/GMT_IMAGE.

weiji14 · 2024-03-29T05:04:55Z

I'm wondering if we should move the _parse_nameunits/_parse_header functions as the GMT_GRID_HEADER class methods, so that we don't have to import these private functions when wrapping GMT_GRID/GMT_IMAGE.

That's not a bad idea. But maybe split the implementation of _parse_header into getter methods like get_attrs(), get_gtype() and so on?

seisman · 2024-03-29T11:25:24Z

But maybe split the implementation of _parse_header into getter methods like get_attrs(), get_gtype() and so on?

Sounds good, but the gtype is determined from the x/y axis attributes. We may need to parse the headers (e.g., the current _parse_header function), then store the returned attrs, dims, gtype and registration into a class attributes (e.g., GMT_GRID_HEADER.nc) and then provide the getter methods which return the requested attributes.

…MT_IMAGE

seisman · 2024-03-31T12:25:29Z

I'm wondering if we should move the _parse_nameunits/_parse_header functions as the GMT_GRID_HEADER class methods, so that we don't have to import these private functions when wrapping GMT_GRID/GMT_IMAGE.
That's not a bad idea. But maybe split the implementation of _parse_header into getter methods like get_attrs(), get_gtype() and so on?

Done in 2bd353a.

weiji14 · 2024-04-01T04:09:06Z

+ def get_registration(self) -> int:
+ """
+ Get the grid registration from the grid header.
+
+ Returns
+ -------
+ registration
+ The grid registration. 0 for gridline and 1 for pixel.
+ """
+ return self.registration


We could just remove this and call self.registration no?

Removed in 79f4c87.

weiji14 · 2024-04-01T04:13:35Z

+ def get_name(self) -> str:
+ """
+ Get the name of the grid from the grid header.
+
+ Returns
+ -------
+ name
+ The name of the grid.
+ """
+ return "z"


Can probably remove this if it is only returning the letter z?

The name defaults to z for 2-D grids and cube for 3-D cubes, but it can be overridden by the varname variable stored in the hidden GMT_GRID_HEADER_HIDDEN data structure. I guess we won't wrap the GMT_GRID_HEADER_HIDDEN data structure now because it is designed to be hidden and may change anytime, but maybe we have to wrap it in the future?

Right, I see that varname is marked as private at https://github.com/GenericMappingTools/gmt/blob/9b93e07982233a31e97b3f8a4ed8df3a3dfa56f9/src/gmt_hidden.h#L161, but it is supposedly holding the name of a NetCDF variable? Maybe we should try to parse it instead of always returning z (or cube)?

As I understand it, varname is only assigned when a netCDF name like input.nc?varname is specified. I guess it's just rare cases.
As mentioned above, to parse varname, we have to wrap the GMT_GRID_HEADER_HIDDEN data structure, which contains more than 50 fields, and the last time it was changed is just a few months ago (GenericMappingTools/gmt@b9d3f2b).

Got it. Let's just leave it unwrapped for now then.

This reverts commit 836c004.

weiji14

Looks good. There's lots of code coverage missing, but that will be covered in #2398 and/or #3128. Might make small changes to the header parsing as we continue on wrapping GMT_GRID, GMT_IMAGE and GMT_CUBE.

GMT_GRID: Add private function _parse_nameunits for parsing long_name…
4bb4e37
… and units from x/y/z_units

seisman added enhancement Improving an existing feature needs review This PR has higher priority and needs review. labels Mar 22, 2024

seisman added this to the 0.12.0 milestone Mar 22, 2024

seisman requested a review from weiji14 March 22, 2024 11:41

michaelgrund reviewed Mar 22, 2024
View reviewed changes

Comment threadpygmt/datatypes/grid.py Outdated
Comment threadpygmt/datatypes/grid.py Outdated

seismanand others added 2 commits March 22, 2024 21:22

Typos
8a514f9
Co-authored-by: Michael Grund <23025878+michaelgrund@users.noreply.github.com>

Add the _parse_heade function
14b3359

seisman changed the title ~~GMT_GRID: Add private function _parse_nameunits for parsing long_name and units from x/y/z_units~~datatypes: Add private functions _parse_nameunits and _parse_header for parsing grid headerMar 22, 2024

seisman changed the title ~~datatypes: Add private functions _parse_nameunits and _parse_header for parsing grid header~~datatypes: Add functions _parse_nameunits and _parse_header for parsing grid headerMar 22, 2024

seisman mentioned this pull request Mar 22, 2024
Wrap GMT's standard data type GMT_GRID for grids #2398
Merged
5 tasks

seisman commented Mar 22, 2024
View reviewed changes

Comment threadpygmt/datatypes/grid.py Outdated
Comment threadpygmt/datatypes/grid.py Outdated

yvonnefroehlich reviewed Mar 22, 2024
View reviewed changes

Comment threadpygmt/datatypes/grid.py Outdated
Comment threadpygmt/datatypes/grid.py Outdated
Comment threadpygmt/datatypes/grid.py Outdated
Comment threadpygmt/datatypes/grid.py Outdated
Comment threadpygmt/datatypes/grid.py Outdated

Apply suggestions from code review
0acc9eb
Co-authored-by: Yvonne Fröhlich <94163266+yvonnefroehlich@users.noreply.github.com>

seisman commented Mar 23, 2024
View reviewed changes

Comment threadpygmt/datatypes/grid.py Outdated

seisman added 2 commits March 25, 2024 07:21

Move GMT_GRID_HEADER and parser functions to pygmt/datatypes/header.py
ab00144

Merge branch 'main' into gmtgrid/parser
b89b2bd

seisman marked this pull request as draft March 25, 2024 23:37

seisman removed the needs review This PR has higher priority and needs review. label Mar 25, 2024

weiji14 reviewed Mar 29, 2024
View reviewed changes

Apply suggestions from code review
b555527
Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>

Move _parse_header as class method
83337e7

seisman added 4 commits March 31, 2024 10:50

Merge branch 'main' into gmtgrid/parser
f50a2be

Patchesb555527
c999b44

Merge branch 'main' into gmtgrid/parser
3482568

Add getter methods to hide techinical details for wrapping GMT_GRID/G…
2bd353a
…MT_IMAGE

seisman changed the title ~~datatypes: Add functions _parse_nameunits and _parse_header for parsing grid header~~GMT_GRID_HEADER: Add getter methods for getting attributes from grid headersMar 31, 2024

Add more global attributes
dfbc2e5

seisman added 2 commits March 31, 2024 20:31

Fix type hints issues
8a2a6e4

Add GMT_GRID_VARNAME_LEN80 used in GMT_CUBE
e3dec88

weiji14 reviewed Apr 1, 2024
View reviewed changes

seisman added 4 commits April 1, 2024 12:40

Add doctests
836c004

Revert "Add doctests"
4ca4533
This reverts commit 836c004.

Decode title, history and description
c5897ba

Change getter methods to properties
79f4c87

weiji14 approved these changes Apr 1, 2024
View reviewed changes

Comment threadpygmt/datatypes/header.py Outdated

seisman changed the title ~~GMT_GRID_HEADER: Add getter methods for getting attributes from grid headers~~GMT_GRID_HEADER: Parse grid header and add grid propertiesApr 1, 2024

seisman added 2 commits April 1, 2024 13:47

Merge branch 'main' into gmtgrid/parser
60808d2

Fix strict=False to strict=True
a4b55ec

seisman marked this pull request as ready for review April 1, 2024 05:50

seisman merged commit 65cc190 into mainApr 1, 2024

seisman deleted the gmtgrid/parser branch April 1, 2024 05:56

Uh oh!

Conversation

seisman commented Mar 22, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seisman commented Mar 29, 2024

Uh oh!

weiji14 commented Mar 29, 2024

Uh oh!

seisman commented Mar 29, 2024

Uh oh!

seisman commented Mar 31, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weiji14 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

seisman commented Mar 22, 2024•
edited
Loading