GMT_GRID_HEADER: Parse grid header and add grid properties #3134

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

pygmt/datatypes/grid.py

Co-authored-by: Michael Grund <[email protected]>

pygmt/datatypes/grid.py

Co-authored-by: Yvonne Fröhlich <[email protected]>

pygmt/datatypes/grid.py

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

pygmt/datatypes/header.py

+        # 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.

pygmt/datatypes/header.py

Co-authored-by: Wei Ji <[email protected]>

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

pygmt/datatypes/header.py

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

pygmt/datatypes/header.py

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

pygmt/datatypes/header.py

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

pygmt/datatypes/header.py

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

pygmt/datatypes/grid.py Outdated Show resolved Hide resolved

pygmt/datatypes/grid.py Outdated Show resolved Hide resolved

seisman and others added 2 commits March 22, 2024 21:22

Typos

8a514f9

Co-authored-by: Michael Grund <[email protected]>

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 header Mar 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 header Mar 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

pygmt/datatypes/grid.py Outdated Show resolved Hide resolved

pygmt/datatypes/grid.py Outdated Show resolved Hide resolved

yvonnefroehlich reviewed Mar 22, 2024

View reviewed changes

Apply suggestions from code review

0acc9eb

Co-authored-by: Yvonne Fröhlich <[email protected]>

seisman commented Mar 23, 2024

View reviewed changes

pygmt/datatypes/grid.py Outdated Show resolved Hide resolved

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 <[email protected]>

Move _parse_header as class method

83337e7

seisman added 4 commits March 31, 2024 10:50

Merge branch 'main' into gmtgrid/parser

f50a2be

Patches b555527

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 headers Mar 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

pygmt/datatypes/header.py Outdated Show resolved Hide resolved

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 properties Apr 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 main Apr 1, 2024
10 of 12 checks passed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GMT_GRID_HEADER: Parse grid header and add grid properties #3134

GMT_GRID_HEADER: Parse grid header and add grid properties #3134

seisman commented Mar 22, 2024 •

edited

Loading

weiji14 Mar 29, 2024

seisman Mar 31, 2024

seisman commented Mar 29, 2024

weiji14 commented Mar 29, 2024

seisman commented Mar 29, 2024

seisman commented Mar 31, 2024

weiji14 Apr 1, 2024

seisman Apr 1, 2024

weiji14 Apr 1, 2024

seisman Apr 1, 2024

weiji14 Apr 1, 2024

seisman Apr 1, 2024

weiji14 Apr 1, 2024

weiji14 left a comment

GMT_GRID_HEADER: Parse grid header and add grid properties #3134

GMT_GRID_HEADER: Parse grid header and add grid properties #3134

Conversation

seisman commented Mar 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seisman commented Mar 29, 2024

weiji14 commented Mar 29, 2024

seisman commented Mar 29, 2024

seisman commented Mar 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiji14 left a comment

Choose a reason for hiding this comment

seisman commented Mar 22, 2024 •

edited

Loading