API cleanup

[d-chambers]

DASCore API Cleanup Plan

(This was an API review by Claude)

Critical Issues

1. update name collision between Patch and Spool
patch.update() creates a new copy with modified fields; spool.update() re-scans the index. These are completely different operations sharing the same name. Since spool.update() is an IO side-effecting operation, consider renaming it to spool.refresh() or spool.rescan() to make the asymmetry clear.

2. The dim vs **kwargs pattern is applied inconsistently
The library has two patterns for specifying which dimension to operate on:

• **kwargs style: pass_filter(time=(1, 100)), pad(time=5), median_filter(time=0.1)
• Explicit dim= style: detrend(dim="time"), normalize(dim="time"), sobel_filter(dim="time")

The implicit rule seems to be: use **kwargs when a per-dimension value accompanies the dimension name, and use dim= when only the dimension name is needed. But sobel_filter breaks this — it uses explicit dim= despite taking extra parameters (mode, cval) just like median_filter. Auditing all proc/ and transform/ functions and enforcing a consistent rule would reduce cognitive load.

3. Too many ways to access coordinate data
Since BaseCoord implements __array__, the transition from coords[name] returning an ndarray to returning a BaseCoord is not actually a breakage for normal use — numpy operations will work transparently. However, the proliferation of access paths is still worth rationalizing:

patch.coords["time"]           # → BaseCoord (post-transition)
patch.coords.get_array("time") # → ndarray
patch.coords.coord_map["time"] # → BaseCoord (internal)
patch.get_coord("time")        # → BaseCoord
patch.get_array("time")        # → ndarray

The preferred public interface should be clearly documented as patch.get_coord() (for the coord object) and patch.get_array() (for raw data), with coords[name] and coords.coord_map treated as internal/advanced access.

---

Moderate Issues

4. detrend(dim, type=...) shadows Python builtin
Rename type to detrend_type or method (consistent with scipy/MNE conventions). The builtin shadowing also confuses static analyzers.

5. No patch.copy() or patch.astype(dtype)
Copying a patch requires patch.update() with no arguments, which is unintuitive. patch.copy() is a one-liner to add. Similarly, there's no patch.astype(float32) — forcing users to do patch.update(data=patch.data.astype(float32)).

6. patch.rolling() is the only method that doesn't return a Patch
Every other Patch method returns a Patch or a scalar, but rolling() returns a roller object. This breaks the fluent chain unexpectedly. The docstring should prominently call this out. Alternatively, the most common uses (.mean(), .std(), .median()) could be provided as direct methods: patch.rolling_mean(dim, window).

7. experiment_id vs acquisition_id mismatch
PatchAttrs uses acquisition_id but PatchFileSummary (the IO index model) uses experiment_id for the same concept. Pick one.

8. dft(dims, ...) accepts a sequence, but most functions use dim (singular)
This inconsistency in parameter naming (dim vs dims) is confusing when scanning function signatures. At minimum, document that dft accepts multiple dims; at best, unify the naming.

---

Minor / Polish Issues

9. patch.viz.spectrogram vs deprecated patch.spectrogram
patch.spectrogram() (the transform) was deprecated in favor of patch.stft(). But patch.viz.spectrogram() (the visualization) is still valid. The same name doing two different things depending on namespace is a documentation hazard — make this distinction very explicit in the deprecation message.

10. Missing common math UFuncs
patch.abs() works, but patch.sqrt(), patch.sin(), patch.cos() aren't exposed. Since PatchUFunc(np.sqrt) etc. is trivial, these should be added — users doing phase/strain conversions frequently need them.

11. No patch.to_xarray() / patch.to_dataframe() export
Interoperability with the broader scientific Python ecosystem (xarray, pandas, scipy) is a common need. Even a basic to_xarray() that maps coords → xr.DataArray would be very useful.

12. Visualization gaps
Only 4 viz functions exist. Missing:

• 1D trace plot (single channel or average trace)
• Amplitude spectrum plot (post-dft)
• These would reduce boilerplate in notebooks significantly.

---

What's Working Well

• The spool pipeline (spool → select → chunk → map) is elegant and composable.
• The patch_function decorator pattern for registering processing steps and tracking history is a well-designed extensibility hook.
• The CoordRange vs CoordArray distinction is smart — it keeps memory usage low for regular DAS grids.
• The units integration throughout (set_units, convert_units, simplify_units) is more principled than most geophysics libraries.
• The deprecation pathway (decorated with removed_in="0.2.0") is clean and user-friendly.

---

Prioritized Action List

Priority	Action
High	Rename spool.update() → spool.refresh() or spool.rescan()
High	Audit and enforce consistent dim= vs **kwargs rule across all proc/transform functions
Low	Document get_coord() / get_array() as the preferred public coord access paths
Medium	Rename detrend(type=...) → detrend(method=...)
Medium	Add patch.copy() and patch.astype(dtype)
Medium	Unify experiment_id / acquisition_id across PatchAttrs and PatchFileSummary
Low	Expose sqrt, sin, cos as UFuncs
Low	Add patch.to_xarray()
Low	Add 1D trace and spectrum viz functions

[d-chambers]

Follow-up discussion: DASDAE scan/index still does too much conversion between flat index rows and PatchSummary.

Current pain point:

• unindexed scan builds PatchSummary from patch groups
• indexed scan reconstructs PatchSummary from flat rows
• index writing still goes through generic scan_to_df / dataframe-style flattening

Possible cleanup:

• make PatchSummary the DASDAE in-memory scan representation
• keep flat dataframe/index rows only at the HDF index boundary
• add DASDAE-local helpers for:
  • patch group -> PatchSummary
  • PatchSummary -> index record
  • index record -> PatchSummary

Goal:
reduce table <-> PatchSummary <-> table churn and keep DASDAE scan/index logic local rather than routing through generic scan helpers.

Once we’re out of Plan mode, I can post that as a PR comment directly.