clib: Add the virtualfile_out method for creating output virtualfile #3057

seisman · 2024-02-20T07:12:13Z

Currently, PyGMT has provided several methods for manipulating virtual files. These methods are:

open_virtualfile: Open a virtual file and close it when done.
virtualfile_from_vectors: Create a virtual file for a table file from 1-D arrays
virtualfile_from_matrix: Create a virtual file for a table file from a 2-D array
virtualfile_from_grid: Create a virtual file for a grid file from a xarray.DataArray object
virtualfile_from_data: Convenience function which wraps virtualfile_from_vectors/virtualfile_from_matrix/virtualfile_from_grid and more
read_virtualfile: Read data from a virtual file

All the virtualfile_from_* methods are for input data. This PR provides a new method virtualfile_out which can create a virtual file for output data. This method is necessary to get rid of temporary files for modules that write tables/grids (#2730).

The same codes are already used in PR #2729 and #2398. This PR is created to make those two PRs smaller and easier to review.

Preview: https://pygmt-dev--3057.org.readthedocs.build/en/3057/api/generated/pygmt.clib.Session.virtualfile_out.html

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 wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
If adding new functionality, add an example to docstrings or tutorials.
Use underscores (not hyphens) in names of Python files and directories.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash command is:

/format: automatically format and lint the code

… files for output

seisman · 2024-02-21T02:39:24Z

We probably need to think more about the function name. Currently, I use virtualfile_to_data, similar to the virtualfile_from_data function. Here, from means input and to means output. virtualfile_from_data sounds like creating a virtual file from an input data, so the function name makes sense, but virtualfile_to_data sounds like converting a virtual file to data, which makes little sense.

Maybe virtualfile_in/virtualfile_out are better names for virtualfile_from_data/virtualfile_to_data? Or even the much shorter version vfile_in/vfile_out?

seisman · 2024-02-26T09:07:09Z

We probably need to think more about the function name. Currently, I use virtualfile_to_data, similar to the virtualfile_from_data function. Here, from means input and to means output. virtualfile_from_data sounds like creating a virtual file from an input data, so the function name makes sense, but virtualfile_to_data sounds like converting a virtual file to data, which makes little sense.

Maybe virtualfile_in/virtualfile_out are better names for virtualfile_from_data/virtualfile_to_data? Or even the much shorter version vfile_in/vfile_out?

Renamed virtualfile_to_data to virtualfile_out in commit a633abf.

virtualfile_from_data will be renamed to virtualfile_in in PR #3068.

seisman · 2024-02-27T12:02:33Z

Ping @GenericMappingTools/pygmt-maintainers for a final review. Otherwise, I'll merge this PR in 24 hours so we can move forward.

weiji14 · 2024-02-27T19:25:28Z

pygmt/clib/session.py

+        Examples
+        --------
+        >>> from pathlib import Path
+        >>> from pygmt.clib import Session
+        >>> from pygmt.datatypes import _GMT_DATASET, _GMT_GRID
+        >>> from pygmt.helpers import GMTTempFile
+        >>>
+        >>> # Create a virtual file for storing the output table.
+        >>> with GMTTempFile(suffix=".txt") as tmpfile:
+        ...     with open(tmpfile.name, mode="w") as fp:
+        ...         print("1.0 2.0 3.0 TEXT", file=fp)
+        ...     with Session() as lib:
+        ...         with lib.virtualfile_out(kind="dataset") as vouttbl:
+        ...             lib.call_module("read", f"{tmpfile.name} {vouttbl} -Td")
+        ...             ds = lib.read_virtualfile(vouttbl, kind="dataset")
+        >>> isinstance(ds.contents, _GMT_DATASET)
+        True
+        >>>
+        >>> # Create a virtual file for storing the output grid.
+        >>> with Session() as lib:
+        ...     with lib.virtualfile_out(kind="grid") as voutgrd:
+        ...         lib.call_module("read", f"@earth_relief_01d_g {voutgrd} -Tg")
+        ...         outgrd = lib.read_virtualfile(voutgrd, kind="grid")
+        >>> isinstance(outgrd.contents, _GMT_GRID)
+        True
+        >>>
+        >>> # Write data to file without creating a virtual file
+        >>> with GMTTempFile(suffix=".nc") as tmpfile:
+        ...     with Session() as lib:
+        ...         with lib.virtualfile_out(
+        ...             kind="grid", fname=tmpfile.name
+        ...         ) as voutgrd:
+        ...             lib.call_module("read", f"@earth_relief_01d_g {voutgrd} -Tg")
+        ...             assert voutgrd == tmpfile.name
+        ...             assert Path(voutgrd).stat().st_size > 0


Should we move this into a new test_clib_virtualfile_out.py file? Two reasons:

There might be other data types like GMT_IMAGE and GMT_CUBE in the future, and the doctest would get long.

Might want to test the output of the virtualfiles more properly using pygmt.info or pygmt.grdinfo.

Should we move this into a new test_clib_virtualfile_out.py file? Two reasons:

There might be other data types like GMT_IMAGE and GMT_CUBE in the future, and the doctest would get long.

The main purpose of these doctests are to show how to use this method and the outpu virtual file in other modules. So I think we can only keep the dataset one only and remove all others.

Might want to test the output of the virtualfiles more properly using pygmt.info or pygmt.grdinfo.

Actually we can't. Because the virtual file is an output (i.e., GMT_OUT), it can't be passed as an input to info/grdinfo. I.e., lib.call_module("info", f"{vouttbl}") won't work.

So I think we can only keep the dataset one only and remove all others.

You can move the grid one to test_clib_virtualfile_out.py

3. Might want to test the output of the virtualfiles more properly using pygmt.info or pygmt.grdinfo.

Actually we can't. Because the virtual file is an output (i.e., GMT_OUT), it can't be passed as an input to info/grdinfo. I.e., lib.call_module("info", f"{vouttbl}") won't work.

Ah ok, maybe not for the virtualfile then. But we should test that saving to a (non-virtual) file works (when fname is used), and try loading from there (i.e. roundtrip). Again, can put this in test_clib_virtualfile_out.py

Should we move this into a new test_clib_virtualfile_out.py file? Two reasons:

There might be other data types like GMT_IMAGE and GMT_CUBE in the future, and the doctest would get long.

The main purpose of these doctests are to show how to use this method and the outpu virtual file in other modules. So I think we can only keep the dataset one only and remove all others.

In commit 9108917, I've removed the doctest for grid and kept the doctests for dataset and fname. I also realized that these doctests are almost the same as the ones in read_virtualfile. This makes sense, because read_virtualfile was implemented to read the data from an output virtualfile. So, I also simplify the doctests in read_virtualfile using the newly added virtualfile_out method.

In other words, we probably need to move the doctests of read_virtualfile into a separate test file in the future.

So I think we can only keep the dataset one only and remove all others.

You can move the grid one to test_clib_virtualfile_out.py

As mentioned in the above comment, the grid one is already doctested in the read_virtualfile method.

Might want to test the output of the virtualfiles more properly using pygmt.info or pygmt.grdinfo.

Actually we can't. Because the virtual file is an output (i.e., GMT_OUT), it can't be passed as an input to info/grdinfo. I.e., lib.call_module("info", f"{vouttbl}") won't work.

Ah ok, maybe not for the virtualfile then. But we should test that saving to a (non-virtual) file works (when fname is used), and try loading from there (i.e. roundtrip). Again, can put this in test_clib_virtualfile_out.py

Done in 888f1c9 but not in a separate test file.

…lfile doctests

clib.Session: Add the virtualfile_to_data method for creating virtual…

a878635

… files for output

seisman force-pushed the virtualfile_to_data branch from 5d278fc to a878635 Compare February 20, 2024 07:19

seisman added the enhancement Improving an existing feature label Feb 20, 2024

seisman added this to the 0.12.0 milestone Feb 20, 2024

seisman added needs review This PR has higher priority and needs review. feature Brand new feature and removed enhancement Improving an existing feature labels Feb 20, 2024

seisman changed the title ~~clib.Session: Add the virtualfile_to_data method for creating virtualfile for output~~ clib.Session: Add the virtualfile_to_data method for creating output virtualfile Feb 20, 2024

Merge branch 'main' into virtualfile_to_data

3ccc70d

seisman requested a review from a team February 25, 2024 03:16

seisman added 3 commits February 25, 2024 13:24

Merge branch 'main' into virtualfile_to_data

c8900d1

Merge branch 'main' into virtualfile_to_data

452f65f

Rename virtualfile_to_data to virtualfile_out

a633abf

seisman changed the title ~~clib.Session: Add the virtualfile_to_data method for creating output virtualfile~~ clib.Session: Add the virtualfile_out method for creating output virtualfile Feb 26, 2024

seisman force-pushed the virtualfile_to_data branch from dfdd3ed to a633abf Compare February 26, 2024 09:11

seisman mentioned this pull request Feb 26, 2024

clib: Rename the virtualfile_from_data method to virtualfile_in #3068

Merged

Merge branch 'main' into virtualfile_to_data

81b8ff0

seisman changed the title ~~clib.Session: Add the virtualfile_out method for creating output virtualfile~~ clib: Add the virtualfile_out method for creating output virtualfile Feb 27, 2024

Improve the docstrings

59527e3

seisman added final review call This PR requires final review and approval from a second reviewer and removed needs review This PR has higher priority and needs review. labels Feb 27, 2024

weiji14 reviewed Feb 27, 2024

View reviewed changes

seisman added 5 commits February 28, 2024 10:01

Merge branch 'main' into virtualfile_to_data

00f23ff

Remove some doctests from virtualfile_out and improve the read_virtua…

9108917

…lfile doctests

Sort alphabetically

ee4cada

Check the content of the output file

888f1c9

Remove unused imports in doctests

519c780

weiji14 approved these changes Feb 28, 2024

View reviewed changes

seisman merged commit 5014591 into main Feb 28, 2024
18 of 19 checks passed

seisman deleted the virtualfile_to_data branch February 28, 2024 04:34

seisman removed the final review call This PR requires final review and approval from a second reviewer label Feb 28, 2024

seisman mentioned this pull request Feb 28, 2024

Wrap GMT's standard data type GMT_DATASET for table inputs #2729

Merged

2 tasks

seisman added enhancement Improving an existing feature and removed feature Brand new feature labels Feb 28, 2024

seisman mentioned this pull request Mar 11, 2024

clib: Add virtualfile_to_dataset method for converting virtualfile to a dataset #3083

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

clib: Add the virtualfile_out method for creating output virtualfile #3057

clib: Add the virtualfile_out method for creating output virtualfile #3057

seisman commented Feb 20, 2024 •

edited by weiji14

Loading

seisman commented Feb 21, 2024

seisman commented Feb 26, 2024 •

edited

Loading

seisman commented Feb 27, 2024

weiji14 Feb 27, 2024

seisman Feb 28, 2024

weiji14 Feb 28, 2024 •

edited

Loading

seisman Feb 28, 2024

seisman Feb 28, 2024 •

edited

Loading

clib: Add the virtualfile_out method for creating output virtualfile #3057

clib: Add the virtualfile_out method for creating output virtualfile #3057

Conversation

seisman commented Feb 20, 2024 • edited by weiji14 Loading

seisman commented Feb 21, 2024

seisman commented Feb 26, 2024 • edited Loading

seisman commented Feb 27, 2024

weiji14 Feb 27, 2024

Choose a reason for hiding this comment

seisman Feb 28, 2024

Choose a reason for hiding this comment

weiji14 Feb 28, 2024 • edited Loading

Choose a reason for hiding this comment

seisman Feb 28, 2024

Choose a reason for hiding this comment

seisman Feb 28, 2024 • edited Loading

Choose a reason for hiding this comment

seisman commented Feb 20, 2024 •

edited by weiji14

Loading

seisman commented Feb 26, 2024 •

edited

Loading

weiji14 Feb 28, 2024 •

edited

Loading

seisman Feb 28, 2024 •

edited

Loading