Skip to content

WIP: Add return_table helper function #1336

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.

Sign up for GitHub

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

Closed
willschlitzer wants to merge 15 commits into from
Closed

WIP: Add return_table helper function #1336

Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
e668ec1
add return_table() to utils.py
willschlitzer Jun 16, 2021
6d8b2e0
add import statements for pandas and numpy
willschlitzer Jun 16, 2021
10f8238
update return_table import
willschlitzer Jun 22, 2021
3546ef6
add exception handling for return_table
willschlitzer Jun 22, 2021
8e59e57
update grdtrack.py and test_grdtrack.py to use return_table
willschlitzer Jun 22, 2021
88832db
format grdtrack gallery example for using df_columns parameter
willschlitzer Jun 22, 2021
3a98346
add test_grdtrack_output_types()
willschlitzer Jun 22, 2021
c8cef5e
add xarray and geopandas output options for return_table()
willschlitzer Jun 25, 2021
46f2cd4
add tests for xarray and GeoDataFrame in test_grdtrack.py
willschlitzer Jun 25, 2021
b2a388f
add tests for xarray and GeoDataFrame as output options for return_ta…
willschlitzer Jun 25, 2021
d7084e5
change assert type to assert isinstance
willschlitzer Jun 25, 2021
935bdaa
move geopandas import in test_grdtrack.py to use importskip
willschlitzer Jun 28, 2021
eae2eef
Merge branch 'master' into table-output-function
willschlitzer Jun 28, 2021
c65fb0e
remove geopandas from available return table options
willschlitzer Jun 29, 2021
61f6126
add test for failure for invalid output type
willschlitzer Jun 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions pygmt/helpers/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
from contextlib import contextmanager

import xarray as xr
import numpy as np
import pandas as pd
from pygmt.exceptions import GMTInvalidInput


Expand Down Expand Up @@ -267,3 +269,48 @@ def args_in_kwargs(args, kwargs):
If one of the required arguments is in ``kwargs``.
"""
return any(arg in kwargs for arg in args)


def return_table(result, data_format, format_parameter, df_columns):
Copy link
Member

@maxrjones maxrjones Jun 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks promising! A few comments:

  1. I think you will need to use this with one of the existing functions so that we can test it out in this PR. In my opinion, grdtrack would be a good option.
  2. My preference would be to add a format_options parameter for return_table. This could take a list, with defaults values including numpy, pandas, and str. This way, if it doesn't for example make sense to return string values then that could not be given as an option in the individual function documentation/implementation.
  3. table-like options for input are now str or numpy.ndarray or pandas.DataFrame or xarray.Dataset or geopandas.GeoDataFrame. It would be nice if all of these were output options too.
  4. I would prefer a more description argument for requested data format than a|d|s. Something like numpy|pandas|str seems more readable.
  5. I think df_columns needs to be optional.

Copy link
Contributor Author

@willschlitzer willschlitzer Jun 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

3. table-like options for input are now `str or numpy.ndarray or 
pandas.DataFrame or xarray.Dataset or geopandas.GeoDataFrame`. 
It would be nice if all of these were output options too.

Sounds good; I'll have to get smarter on the last two but I don't see why it should be a problem.

4. I would prefer a more description argument for requested data format than **a**|**d**|**s**. 
Something like `numpy`|`pandas`|`str` seems more readable.

I like the idea of keeping it short, especially when there is a default option (I anticipate it being a numpy array) and the strings are not also the same word as Python modules or variable types. But I understand how the single letters could be confusing.

5. I think `df_columns` needs to be optional.

Since this is a helper function, I envisioned that the argument for df_columns would be set up in the GMT function that is using it, such as using ["x", "y", "z"] when calling this function inside of grd2xyz.

Copy link
Contributor Author

@willschlitzer willschlitzer Jun 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

3. table-like options for input are now `str or numpy.ndarray or pandas.DataFrame 
or xarray.Dataset or geopandas.GeoDataFrame`. 
It would be nice if all of these were output options too.

Added in c8cef5e

Copy link
Member

@weiji14 weiji14 Jun 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I would prefer a more description argument for requested data format than a|d|s.
    Something like numpy|pandas|str seems more readable.

I like the idea of keeping it short, especially when there is a default option (I anticipate it being a numpy array) and the strings are not also the same word as Python modules or variable types. But I understand how the single letters could be confusing.

I'll have to agree with Meghan that long descriptive names like numpy|pandas|str are preferable 🙂

r"""
Take the table output from the GMT API and return it as either a string,
array, or DataFrame.

Parameters
----------
result : str
The table returned from the GMT API as a string.
data_format : str
A single-letter string that specifies requested data format of the
table.
**a** : numpy array
**d** : pandas DataFrame
**s** : string
format_parameter : str
The name of the parameter used to specify the data format in the
pygmt function. This name is used when raising the GMTInvalidInput
error to ensure module-specific parameters are consistent with the
error raised.
df_columns : list
The column names of the returned pandas DataFrame.
"""

if data_format == "s":
return result
data_list = []
for string_entry in result.strip().split("\n"):
float_entry = []
string_list = string_entry.strip().split()
for i in string_list:
float_entry.append(float(i))
data_list.append(float_entry)
data_array = np.array(data_list)
if data_format == "a":
result = data_array
elif data_format == "d":
result = pd.DataFrame(data_array, columns=df_columns)
else:
raise GMTInvalidInput(
f"""Must specify {format_parameter} as either a, d, or s."""
)
return result