Skip to content
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.

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

Remove the deprecated load_japan_quakes function (deprecated since v0.6.0) #2301

Merged
merged 5 commits into from
Jan 5, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
1 change: 0 additions & 1 deletion doc/api/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ Use :func:`pygmt.datasets.load_sample_data` instead.

datasets.load_fractures_compilation
datasets.load_hotspots
datasets.load_japan_quakes
datasets.load_mars_shape
datasets.load_ocean_ridge_points
datasets.load_sample_bathymetry
Expand Down
1 change: 0 additions & 1 deletion pygmt/datasets/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
list_sample_data,
load_fractures_compilation,
load_hotspots,
load_japan_quakes,
load_mars_shape,
load_ocean_ridge_points,
load_sample_bathymetry,
Expand Down
61 changes: 21 additions & 40 deletions pygmt/datasets/samples.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,18 @@ def load_sample_data(name):
"bathymetry": load_sample_bathymetry,
"fractures": load_fractures_compilation,
"hotspots": load_hotspots,
"japan_quakes": load_japan_quakes,
"mars_shape": load_mars_shape,
"ocean_ridge_points": load_ocean_ridge_points,
"usgs_quakes": load_usgs_quakes,
}

# Dictionary of private load functions
load_func = {
"rock_compositions": _load_rock_sample_compositions,
"earth_relief_holes": _load_earth_relief_holes,
"japan_quakes": _load_japan_quakes,
"maunaloa_co2": _load_maunaloa_co2,
"notre_dame_topography": _load_notre_dame_topography,
"rock_compositions": _load_rock_sample_compositions,
}

if name in load_func_old:
Expand All @@ -95,51 +95,32 @@ def load_sample_data(name):
return data


def load_japan_quakes(**kwargs):
def _load_japan_quakes():
"""
(Deprecated) Load a table of earthquakes around Japan as a
pandas.DataFrame.

.. warning:: Deprecated since v0.6.0. This function has been replaced with
``load_sample_data(name="japan_quakes")`` and will be removed in
v0.9.0.

Data is from the NOAA NGDC database. This is the ``@tut_quakes.ngdc``
dataset used in the GMT tutorials.
Comment on lines -107 to -108
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should keep this background information of the data?


The data are downloaded to a cache directory (usually ``~/.gmt/cache``) the
first time you invoke this function. Afterwards, it will load the data from
the cache. So you'll need an internet connection the first time around.
Load a table of earthquakes around Japan as a pandas.DataFrame.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Load a table of earthquakes around Japan as a pandas.DataFrame.
Load a table of earthquakes around Japan as a pandas.DataFrame.
Data is from the NOAA NGDC database. This is the "@tut_quakes.ngdc"
dataset used in the GMT tutorials.

Copy link
Member Author

Choose a reason for hiding this comment

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

@yvonnefroehlich Thanks for your comments.

Data is from the NOAA NGDC database.

I've added this sentence back in 92bcec3.

This is the "@tut_quakes.ngdc" dataset used in the GMT tutorials.

I don't think this sentence would provide any meaningful information. The file name is easy to get from the function and we all know that these dataset are used in the tutorials.

Copy link
Member

@yvonnefroehlich yvonnefroehlich Jan 5, 2023

Choose a reason for hiding this comment

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

I was not complete sure about this, as in the PRs #2306 and #2308 this is not removed but in the PRs #2301 (this PR), #2303, and #2305 it is removed. I have no big preference on this, but it should be consistent for all functions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's discuss it in this PR and then update other PRs if necessary.

@michaelgrund and @willschlitzer what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I'm with @seisman since this information is not really required at this point in my opinion.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, then we can merge this PR.

seisman marked this conversation as resolved.
Show resolved Hide resolved

Returns
-------
data : pandas.DataFrame
The data table. Columns are year, month, day, latitude, longitude,
depth (in km), and magnitude of the earthquakes.
The data table. The column names are "year", "month", "day",
"latitude", "longitude", "depth_km", and "magnitude" of the
earthquakes.
"""

if "suppress_warning" not in kwargs:
warnings.warn(
"This function has been deprecated since v0.6.0 and will be "
"removed in v0.9.0. Please use "
"load_sample_data(name='japan_quakes') instead.",
category=FutureWarning,
stacklevel=2,
)

fname = which("@tut_quakes.ngdc", download="c")
data = pd.read_csv(fname, header=1, sep=r"\s+")
data.columns = [
"year",
"month",
"day",
"latitude",
"longitude",
"depth_km",
"magnitude",
]

return data
return pd.read_csv(
fname,
header=1,
delim_whitespace=True,
names=[
"year",
"month",
"day",
"latitude",
"longitude",
"depth_km",
"magnitude",
],
)
Comment on lines +112 to +125
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI, I also made two changes when reading the dataset:

  1. Use delim_whitespace=True instead of sep=r"\s+". They're the same but I think delim_whitespace=True is easier to understand (just my own preference).
  2. Use the names parameter in the read_csv function, instead of changing the columns later.



def load_ocean_ridge_points(**kwargs):
Expand Down
17 changes: 0 additions & 17 deletions pygmt/tests/test_datasets_samples.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from pygmt.datasets import (
load_fractures_compilation,
load_hotspots,
load_japan_quakes,
load_mars_shape,
load_ocean_ridge_points,
load_sample_bathymetry,
Expand All @@ -26,22 +25,6 @@ def test_load_sample_invalid():


def test_japan_quakes():
"""
Check that the dataset loads without errors.
"""
with pytest.warns(expected_warning=FutureWarning) as record:
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like test_japan_quakes and test_load_sample_data are the same. Should one of them be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right. I removed in a35adc6.

data = load_japan_quakes()
assert len(record) == 1
assert data.shape == (115, 7)
assert data["year"].min() == 1987
assert data["year"].max() == 1988
assert data["month"].min() == 1
assert data["month"].max() == 12
assert data["day"].min() == 1
assert data["day"].max() == 31


def test_load_sample_data():
"""
Check that the dataset loads without errors.
"""
Expand Down