Standardize parameter name for table data inputs

Description of the desired feature

Originally posted by @meghanrjones in #1441 (comment)

This is somewhat related - I am not sure if it's worth a deprecation but I think we should decide on whether the parameter for tabular input is data or table for all modules moving forward. Currently, this is the distribution:

data parameter: surface, wiggle, velo, rose, plot3d, plot, contour
table parameter: blockmedian, blockmean, histogram
other: grdtrack (points), text (textfiles)

It would also be worthwhile to set a consistent order for the positional arguments x, y, [z], data, [outfile]

Are you willing to help implement and maintain this feature? Discuss first

One option to reduce the amount of deprecation notices is to keep two parameters, but with this rule:

data - use for the fig.* plotting functions (e.g. fig.plot, fig.contour, etc)
table - use for the data processing functions (e.g. blockmean, histogram, etc)

This way, we'll only need to do the renaming for surface ('data' -> 'table'), grdtrack ('points' -> 'table') and text ('textfiles' -> 'data'). I have some reservations with renaming text's 'textfiles' to 'data' since it's a bit non-standard (and need to wait for #1121), but otherwise, what do people think?

One option to reduce the amount of deprecation notices is to keep two parameters, but with this rule:
* `data` - use for the `fig.*` plotting functions (e.g. `fig.plot`, `fig.contour`, etc)

* `table` - use for the data processing functions (e.g. `blockmean`, `histogram`, etc)
This way, we'll only need to do the renaming for surface ('data' -> 'table'), grdtrack ('points' -> 'table') and text ('textfiles' -> 'data'). I have some reservations with renaming text's 'textfiles' to 'data' since it's a bit non-standard (and need to wait for #1121), but otherwise, what do people think?

One complication is that many modules can be plotting or processing (e.g., histogram is mostly plotting but can return statistics with -I). My preference would be to use data moving forward and, if needed, deprecate table to data in blockmean, blockmedian, and histogram. grdtrack and text are non-standard enough for different names in my opinion.

One complication is that many modules can be plotting or processing (e.g., histogram is mostly plotting but can return statistics with -I).

Those types of dual-purpose functions (e.g. histogram, rose, solar) might end up having two APIs, depending on how we decide at #896. E.g. a fig.histogram for plotting and pygmt.histogram for extracting data.

My preference would be to use data moving forward and, if needed, deprecate table to data in blockmean, blockmedian, and histogram. grdtrack and text are non-standard enough for different names in my opinion.

Ok, sounds like we need a vote. So the options are:

🎉 Use data only for all functions, as per Meghan at Standardize parameter name for table data inputs #1479 (comment)
🚀 Use table only for all functions
👀 Use data for plotting functions, table for processing functions as per Wei Ji at Standardize parameter name for table data inputs #1479 (comment).
😕 Some other alternative - leave a comment below.

Place your vote on this comment. Only one vote per person 😛

Can we also set the conventional order for the positional arguments as data/table, x, y, [z] or x, y, [z], data/table? My opinion is that this isn't worth deprecating any current syntax, but that #1434, #1429, #1418, #1379, #731 and #1491 should have the same order (if both data/table and x/y[/z] will be supported).

Here's the current count:

x, y, [z], data/table - (5 current functions) contour, plot3d, wiggle, surface, plot (Used in PRs Wrap nearneighbor #1379
data/table, [outfile], x, y, [z] - (2 current functions) blockmean, blockmedian (Used in PRs Wrap triangulate #731, Wrap blockmode #1456)
only data/table with no x/y[/z] option - (2 current functions and 2 functions in v0.5.0) histogram, sphdistance, xyz2grd, info (Used in PRs Wrap gmtselect #1429, Wrap sph2grd #1434, Wrap sphinterpolate #1418, Expand table-like input options for sphdistance #1491)

Can we also set the conventional order for the positional arguments as data/table, x, y, [z] or x, y, [z], data/table?

Yes, good point. I would say just go with data, x, y, [z], so people can do pygmt.somefunction(data) if data is a pandas.DataFrame/geopandas.GeoDataFrame or any table-like input, similar to other Python-like plotting/processing code like plt.imshow. In this case, we would need to modify:

x, y, [z], data/table - (5 current functions) contour, plot3d, wiggle, surface, plot (Used in PRs Wrap nearneighbor #1379

Debating on whether we should go through a deprecation cycle (2 releases) for the x/y/z/data -> data/x/y/z parameter ordering change. Personally I try to always do parameter=arg in my code, but many people just pass in the arguments directly. So maybe best to raise a warning to give people time to switch over 🙃

Can we also set the conventional order for the positional arguments as data/table, x, y, [z] or x, y, [z], data/table?

Yes, good point. I would say just go with data, x, y, [z], so people can do pygmt.somefunction(data) if data is a pandas.DataFrame/geopandas.GeoDataFrame or any table-like input, similar to other Python-like plotting/processing code like plt.imshow. In this case, we would need to modify:

x, y, [z], data/table - (5 current functions) contour, plot3d, wiggle, surface, plot (Used in PRs Wrap nearneighbor #1379

Debating on whether we should go through a deprecation cycle (2 releases) for the x/y/z/data -> data/x/y/z parameter ordering change. Personally I try to always do parameter=arg in my code, but many people just pass in the arguments directly. So maybe best to raise a warning to give people time to switch over 🙃

Sounds good. If we will make the change for the functions that have x/y/z/data, I think we should raise a warning.

Just want to confirm that we have reached a consensus that we will use data and the order data, x, y, z for table inputs. If yes, then we can start to update these functions.

If we will make the change for the functions that have x/y/z/data, I think we should raise a warning.

It's unclear to me how to raise a warning for this case. For users who do x=x, y=y, z=z, the parameter order doesn't matter, thus the warning is unnecessary and may be confusing. For users who do x, y, z, how can know they pass arguments directly without giving parameters?

Just want to confirm that we have reached a consensus that we will use data and the order data, x, y, z for table inputs. If yes, then we can start to update these functions.

Yes, I think so.

If we will make the change for the functions that have x/y/z/data, I think we should raise a warning.

It's unclear to me how to raise a warning for this case. For users who do x=x, y=y, z=z, the parameter order doesn't matter, thus the warning is unnecessary and may be confusing. For users who do x, y, z, how can know they pass arguments directly without giving parameters?

If the user passes in x, y, z, then len(args) will be at least 3. If the user passes in x=x, y=y, z=y, then len(args) will be 0, using a decorator similar to deprecate_parameter.

Here are a list of modules that may need to rename table to data or reorder parameters like data, x, y, z:

blockm.py: blockm(block_method, table, outfile, x, y, z, **kwargs) pygmt.blockm*: Reorder input parameters to 'data, x, y, z' #1565
blockm.py: blockmean(table=None, outfile=None, *, x=None, y=None, z=None, **kwargs) pygmt.blockm*: Reorder input parameters to 'data, x, y, z' #1565
blockm.py: blockmedian(table=None, outfile=None, *, x=None, y=None, z=None, **kwargs) pygmt.blockm*: Reorder input parameters to 'data, x, y, z' #1565
blockm.py: blockmode(table=None, outfile=None, *, x=None, y=None, z=None, **kwargs) pygmt.blockm*: Reorder input parameters to 'data, x, y, z' #1565
contour.py: contour(self, x=None, y=None, z=None, data=None, **kwargs) Figure.contour: Reorder input parameters to 'data, x, y, z' #1561
grdtrack.py: grdtrack(points, grid, newcolname=None, outfile=None, **kwargs)
histogram.py: histogram(self, table, **kwargs) Figure.histogram: Deprecate parameter "table" to "data" (remove in v0.7.0) #1540.
info.py: info(table, **kwargs) (pygmt.info: Deprecate parameter "table" to "data" (remove in v0.7.0) #1538)
plot.py: plot(self, x=None, y=None, data=None, size=None, direction=None, **kwargs) Figure.plot: Reorder input parameters to "data, x, y" #1547
plot3d.py: plot3d(self, x=None, y=None, z=None, data=None, size=None, direction=None, **kwargs) Figure.plot3d: Reorder input parameters to 'data, x, y, z' #1560
rose.py: rose(self, length=None, azimuth=None, data=None, **kwargs) Figure.rose: Reorder input parameters to 'data, length, azimuth' #1546
sphdistance.py: sphdistance(table, **kwargs) (Expand table-like input options for sphdistance #1491)
surface.py: surface(x=None, y=None, z=None, data=None, **kwargs) pygmt.surface: Reorder input parameters to 'data, x, y, z' #1562
text.py: text_(self, textfiles=None, x=None, y=None, position=None, text=None, angle=None, font=None, justify=None, **kwargs)
velo.py: velo(self, data=None, **kwargs)
wiggle.py: wiggle(self, x=None, y=None, z=None, data=None, **kwargs) (Figure.wiggle: Reorder input parameter to 'data, x, y, z' #1548)
x2sys_cross.py: x2sys_cross(tracks=None, outfile=None, **kwargs)
xyz2grd.py: xyz2grd(table, **kwargs) (pygmt.xyz2grd: Rename the parameter 'table' to 'data' #1545)

If the user passes in x, y, z, then len(args) will be at least 3. If the user passes in x=x, y=y, z=y, then len(args) will be 0, using a decorator similar to deprecate_parameter.

It looks like a good solution. I'll work on it.

If the user passes in x, y, z, then len(args) will be at least 3. If the user passes in x=x, y=y, z=y, then len(args) will be 0, using a decorator similar to deprecate_parameter.

I'm rethinking the decorator implemented in #1541.

We're changing the parameter order from x, y, [z], data to data, x, y, [z]. Before making the change, users can pass positional parameters x, y, **kwargs or x, y, z, **kwargs. After making the change, users still can pass positional parameters, but they can only use it in this way data, **kwargs.

Thus, actually, we should raise the warning only if len(args) > 1 (i.e., x, y or x, y, z), for non-plotting functions.

For plotting functions, there is always the self parameter, thus, we should check if len(args) > 2 instead.

If the user passes in x, y, z, then len(args) will be at least 3. If the user passes in x=x, y=y, z=y, then len(args) will be 0, using a decorator similar to deprecate_parameter.

I'm rethinking the decorator implemented in #1541.

We're changing the parameter order from x, y, [z], data to data, x, y, [z]. Before making the change, users can pass positional parameters x, y, **kwargs or x, y, z, **kwargs. After making the change, users still can pass positional parameters, but they can only use it in this way data, **kwargs.

Thus, actually, we should raise the warning only if len(args) > 1 (i.e., x, y or x, y, z), for non-plotting functions.

For plotting functions, there is always the self parameter, thus, we should check if len(args) > 2 instead.

In addition to len(args), we might actually want to check the type or shape of args[0] (for processing) or args[1] (for plotting). For example, in #1546 someone could pass in data, length, azimuth as positional arguments and should not get a warning even though len(args>2).

For example, in #1546 someone could pass in data, length, azimuth as positional arguments and should not get a warning even though len(args>2).

Users can pass data, data=data, or length=length, azimuth=azimuth, but they can't do data, length, azimuth because we have the data_kind function to make sure that users cannot pass both data and x,y.

For example, in #1546 someone could pass in data, length, azimuth as positional arguments and should not get a warning even though len(args>2).

Users can pass data, data=data, or length=length, azimuth=azimuth, but they can't do data, length, azimuth because we have the data_kind function to make sure that users cannot pass both data and x,y.

OK, that was a bad example. An alternate example for plot: data, None, None, size. I don't know why anyone would want to do this, but they could.

We're changing the parameter order from x, y, [z], data to data, x, y, [z]. Before making the change, users can pass positional parameters x, y, **kwargs or x, y, z, **kwargs. After making the change, users still can pass positional parameters, but they can only use it in this way data, **kwargs.

Thus, actually, we should raise the warning only if len(args) > 1 (i.e., x, y or x, y, z), for non-plotting functions.

For plotting functions, there is always the self parameter, thus, we should check if len(args) > 2 instead.

I've implemented it in 95fa173 as part of PR #1548.

For example, in #1546 someone could pass in data, length, azimuth as positional arguments and should not get a warning even though len(args>2).

Users can pass data, data=data, or length=length, azimuth=azimuth, but they can't do data, length, azimuth because we have the data_kind function to make sure that users cannot pass both data and x,y.

OK, that was a bad example. An alternate example for plot: data, None, None, size. I don't know why anyone would want to do this, but they could.

I prefer NOT to check these complicated cases:

Although users can pass data, None, None, size, I believe it's very rare. The current decorator only raise a warning, so users script won't stop working.
The decorator is temporary, and likely will be removed after v0.7.0. It's six months if we strictly following the 3-month release cycle.

After #1565 is merged, all functions will use consistent parameter name data, and consistent parameter order data, x, y, [z]. There are few exceptions (see https://www.pygmt.org/dev/api/index.html):

image(self, imagefile, ...)
legend(self, spec=None, ...)
meca(self, spec, scale, ...)
rose(data=None, length=None, azimuth=None, ...)
text(textfiles=None, x=None, y=None, ...)
grdtrack(points, grid, ...)
x2sys_cross(tracks=None, outfile=None, ...)

I think these exceptions are reasonable and don't need more changes. Thus, we can close the issue after #1565 is merged. Agree?

Rename the parameter 'table' to 'data'

Wrap triangulate (#731)

Wrap triangulate (GenericMappingTools#731)

Navigation Menu

Repository navigation

Standardize parameter name for table data inputs #1479

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Standardize parameter name for table data inputs #1479

Description

Activity

weiji14 commented on Sep 1, 2021

maxrjones commented on Sep 1, 2021

weiji14 commented on Sep 3, 2021

maxrjones commented on Sep 11, 2021

weiji14 commented on Sep 15, 2021

maxrjones commented on Sep 15, 2021

seisman commented on Sep 22, 2021

maxrjones commented on Sep 22, 2021

seisman commented on Sep 23, 2021

seisman commented on Sep 24, 2021

seisman commented on Sep 27, 2021

maxrjones commented on Sep 27, 2021

seisman commented on Sep 27, 2021

maxrjones commented on Sep 27, 2021

seisman commented on Sep 28, 2021

seisman commented on Oct 3, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions