add get_name patch/spool method #471

d-chambers · 2024-12-21T07:28:51Z

Description

This PR implements a get_name method for the patch and the spool to return a series of names. The names are either generated by columns provided to the function (which has sensible defaults to maintain the old behavior) or by existing columns such as name or path.

This PR supersede #461.

Checklist

I have (if applicable):

referenced the GitHub issue this PR closes.
documented the new feature with docstrings or appropriate doc page.
included a test. See testing guidelines.
your name has been added to the contributors page (docs/contributors.md).
added the "ready_for_review" tag once the PR is ready to be reviewed.

codecov · 2024-12-21T07:31:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.85%. Comparing base (2138fee) to head (6044f67).
Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #471   +/-   ##
=======================================
  Coverage   99.85%   99.85%           
=======================================
  Files         115      115           
  Lines        9507     9548   +41     
=======================================
+ Hits         9493     9534   +41     
  Misses         14       14

Flag	Coverage Δ
unittests	`99.85% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

d-chambers · 2024-12-24T01:49:59Z

@ahmadtourei, mind taking this for a spin to see if it behaves how you wanted ?

dascore/utils/patch.py

ahmadtourei · 2024-12-24T05:01:00Z

dascore/utils/patch.py

+    Examples
+    --------
+    >>> import dascore as dc
+    >>> from dascore.utils.patch import get_patch_names
+    >>> patch = dc.get_example_patch()
+    >>> name = get_patch_names(patch)


name is:

0 DAS_______random__2017_09_18__2017_09_18T00_00_07 Name: network, dtype: object

We get DAS_______random__2017_09_18__2017_09_18T00_00_07 using name[0] because the function returns a <class 'pandas.core.series.Series'>. Is there a specific reason that it returns a pandas series with Name: network, dtype: object (a little confusing) instead of the names in string?

Yes, for the function to work with a spool it needs to return multiple names. I avoided returning multiple types (based on if the input is a Patch or not) from get_patch_names as that is a bit of an anti-pattern.

I understand the confusion though. I will add get_patch_name as a method of Patch that will do this. I think with the different name (get_patch_name vs get_patch_names) it will be clear.

ahmadtourei · 2024-12-24T05:08:28Z

dascore/utils/patch.py

+    patch_data: pd.DataFrame | dc.Patch | dc.BaseSpool,
+    prefix="DAS",
+    attrs=("network", "station", "tag"),
+    coords=("time",),
+    sep="__",


need test(s) for optional inputs . For example, get_patch_names(patch, patch) does not raise an error.

ahmadtourei · 2024-12-24T05:12:50Z

tests/conftest.py

+@pytest.fixture(scope="session")
+def random_spool_directory(tmp_path_factory):
+    """A directory with a few patch files."""
+    out = Path(tmp_path_factory.mktemp("one_file_file_spool"))
+    spool = ex.get_example_spool("random_das")
+    out_path = ex.spool_to_directory(spool, path=out)
+    return dc.spool(out_path).update()


It would be nice to have this as an example in dascore.examples as all of the example spools are memory spools.

ahmadtourei · 2024-12-24T05:18:36Z

dascore/utils/patch.py

+    def _get_filename(path_ser):
+        """Get the file name from a path series."""
+        ser = path_ser.astype(str)
+        file_names = [x[-1].split(".")[0] for x in ser.str.split("/")]


How about we introduce an optional argument (such as keep_extension) so the user can decide whether to get patch's name with or without file extension? I can work on this as I already implemented this in my version #461. What do you think?

ahmadtourei

Looks good! I have posted my comments below.

One general comment that I have is because this can be a very useful function, it would be great to create a shortcut for it (something like patch.name and spool.name so the user can quickly get name(s) in string format. Then we can include it in the Shortcuts section of dascore/docs/tutorial/patch.qmd. I assume users with limited DASCore experience cannot quickly locate it in dascore.utils.patch.

d-chambers · 2024-12-24T16:17:19Z

I assume users with limited DASCore experience cannot quickly locate it in dascore.utils.patch.

I agree. I think it should be a patch/spool method. Because there are optional parameters though, probably best not to make it an attribute (because then the users are always stuck with the defaults).

d-chambers · 2024-12-26T17:58:22Z

Thanks @ahmadtourei for the great review. I have addressed all of your suggestions/concerns. Anything else you want to see in here? I also included a fix for the doc build so I am keen to merge this soon to see if the fix works.

ahmadtourei · 2024-12-29T19:56:37Z

dascore/examples.py

@@ -451,6 +451,24 @@ def random_spool(time_gap=0, length=3, time_min=np.datetime64("2020-01-03"), **k
    return dc.spool(out)


+@register_func(EXAMPLE_SPOOLS, key="radom_directory_das")


ahmadtourei · 2024-12-29T20:01:31Z

docs/tutorial/patch.qmd

+The "name" of the patch, which is the filename that would be used if the patch were saved to disk by DASCore, can be generated via the [`Spool.get_patch_names`](`dascore.BaseSpool.get_patch_names`) to get a series of the names of the managed patches, or [`Patch.get_patch_name`](`dascore.Patch.get_patch_name`) to get a string of the patch name.
+
+```python 
+import dascore as dc
+
+patch = dc.get_example_patch()
+spool = dc.get_example_spool()
+
+patch_name = patch.get_patch_name()
+patch_names = spool.get_patch_names()
+```


ahmadtourei · 2024-12-29T20:21:10Z

Thanks @ahmadtourei for the great review. I have addressed all of your suggestions/concerns. Anything else you want to see in here? I also included a fix for the doc build so I am keen to merge this soon to see if the fix works.

Thanks, Derrick, for your work on this PR! It looks great. The only new comment I have is a typo I listed below.

d-chambers mentioned this pull request Dec 24, 2024

patch_name method for dir spools #461

Closed

5 tasks

d-chambers added 4 commits December 23, 2024 17:22

add get_name patch/spool method

a3e2fb9

rework specical get_name cases

53748a3

rename to get_patch_names

b90bcbd

lint

70e2e93

d-chambers force-pushed the get_name branch from fb0707a to 70e2e93 Compare December 24, 2024 01:24

d-chambers added the ready_for_review PR is ready for review label Dec 24, 2024

ahmadtourei reviewed Dec 24, 2024

View reviewed changes

dascore/utils/patch.py Outdated Show resolved Hide resolved

ahmadtourei reviewed Dec 24, 2024

View reviewed changes

ahmadtourei requested changes Dec 24, 2024

View reviewed changes

d-chambers added 4 commits December 24, 2024 12:21

fix doc build

ec69270

strip_extension test

1b0ab1b

merge master

eb7acdc

fix test typo

b9137ac

ahmadtourei reviewed Dec 29, 2024

View reviewed changes

ahmadtourei approved these changes Dec 29, 2024

View reviewed changes

fix typo

6044f67

d-chambers merged commit 4f48bd6 into master Dec 30, 2024
18 checks passed

d-chambers deleted the get_name branch December 30, 2024 18:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add get_name patch/spool method #471

add get_name patch/spool method #471

d-chambers commented Dec 21, 2024

codecov bot commented Dec 21, 2024 •

edited

Loading

d-chambers commented Dec 24, 2024

ahmadtourei Dec 24, 2024 •

edited

Loading

d-chambers Dec 24, 2024

ahmadtourei Dec 24, 2024 •

edited

Loading

ahmadtourei Dec 24, 2024

ahmadtourei Dec 24, 2024 •

edited

Loading

ahmadtourei left a comment

d-chambers commented Dec 24, 2024

d-chambers commented Dec 26, 2024

ahmadtourei Dec 29, 2024

ahmadtourei Dec 29, 2024

ahmadtourei commented Dec 29, 2024

		@@ -451,6 +451,24 @@ def random_spool(time_gap=0, length=3, time_min=np.datetime64("2020-01-03"), **k
		return dc.spool(out)


		@register_func(EXAMPLE_SPOOLS, key="radom_directory_das")

add get_name patch/spool method #471

add get_name patch/spool method #471

Conversation

d-chambers commented Dec 21, 2024

Description

Checklist

codecov bot commented Dec 21, 2024 • edited Loading

Codecov Report

d-chambers commented Dec 24, 2024

ahmadtourei Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

d-chambers Dec 24, 2024

Choose a reason for hiding this comment

ahmadtourei Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

ahmadtourei Dec 24, 2024

Choose a reason for hiding this comment

ahmadtourei Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

ahmadtourei left a comment

Choose a reason for hiding this comment

d-chambers commented Dec 24, 2024

d-chambers commented Dec 26, 2024

ahmadtourei Dec 29, 2024

Choose a reason for hiding this comment

ahmadtourei Dec 29, 2024

Choose a reason for hiding this comment

ahmadtourei commented Dec 29, 2024

codecov bot commented Dec 21, 2024 •

edited

Loading

ahmadtourei Dec 24, 2024 •

edited

Loading

ahmadtourei Dec 24, 2024 •

edited

Loading

ahmadtourei Dec 24, 2024 •

edited

Loading