tests/test_core/test_attrs.py (1)

261-270: Add assertions for immutability and type of result (nice-to-have).

Also assert the original is unchanged and the returned object is PatchAttrs.

 def test_simple_drop(self):
   """Ensure a single attr can be dropped."""
   attrs = PatchAttrs(bob=1, bill=2, sue="Z")
   new = dict(attrs.drop("bob", "bill"))
+  # original unchanged
+  orig = dict(attrs)
+  assert "bob" in orig and "bill" in orig and "sue" in orig
+  # type preserved
+  out = attrs.drop("bob", "bill")
+  assert isinstance(out, PatchAttrs)
   assert "bob" not in new and "bill" not in new
   assert "sue" in new

dascore/core/coords.py (1)

766-769: Doc improvement is helpful; clarify behavior for non-positive values.

Current doc claims results are “always ≥ 1”, but the implementation can return 0 or negatives (e.g., samples=True with value≤0, or value < min for time-like). Consider clarifying the doc or clamping the return to at least 1.

-Therefore, the output will always be greater or equal to 1.
+If value is positive, the result will be >= 1. (Callers may clamp to >=1 if needed.)

Or enforce it in code:

-        return samples
+        return max(1, samples)

dascore/core/attrs.py (1)

250-256: drop() works; minor simplification and doc tweak (nit).

Small simplification avoids an intermediate set and clarifies that a new instance is returned.

-    def drop(self, *args):
-        """Drop specific keys if they exist."""
-        contents = dict(self)
-        ok_to_keep = set(contents) - set(args)
-        out = {i: v for i, v in contents.items() if i in ok_to_keep}
-        return self.__class__(**out)
+    def drop(self, *args):
+        """Return a new PatchAttrs without the specified keys (if present)."""
+        contents = dict(self)
+        out = {k: v for k, v in contents.items() if k not in set(args)}
+        return self.__class__(**out)

dascore/utils/time.py (2)

405-408: Correct is_datetime64 docstring.

It currently says “timedelta64”; should be “datetime64”.

-def is_datetime64(obj):
-    """Determine if an object represents a timedelta64 dtype or value(s)."""
+def is_datetime64(obj):
+    """Determine if an object represents a datetime64 dtype or value(s)."""

---

383-413: Optional: treat Python datetime/timedelta scalars as time-like.

Extend _is_dtype to also accept datetime.datetime and datetime.timedelta when inspecting object-dtype arrays. Note that is_datetime64/is_timedelta64 are invoked across several modules—transform/fourier.py, units.py, utils/chunk.py, core/coords.py—and exercised by tests in test_utils/test_time.py and test_core/test_coords.py. Ensure you update those call sites and corresponding tests to cover the new behavior.

dascore/transform/fourier.py (2)

461-462: Consider preserving data_units in STFT attributes.

The STFT transformation should update the data units to reflect the transformation. Currently, data_units is not updated in the attributes.

Consider adding data units transformation similar to how it's handled in the DFT:

new_attrs["data_units"] = _get_data_units_from_dims(patch, [dim], mul)

---

543-544: Consider using existing shape validation instead of assertion.

The assertion on Line 543 could be replaced with a more informative error if shapes don't match.

-    assert new_data.shape == cm.shape
+    if new_data.shape != cm.shape:
+        msg = f"Data shape {new_data.shape} doesn't match coordinate shape {cm.shape}"
+        raise PatchError(msg)

dascore/proc/coords.py (2)

561-563: Fix typo in docs

“diemsnions” → “dimensions”.

Apply:

-        Can also include ... to indicate diemsnions that should be left
+        Can also include ... to indicate dimensions that should be left
         alone.

---

566-577: Standardize examples and align with project style

Use the common alias import dascore as dc as used in other docstrings in this file.

Apply:

-    >>> import dascore # import dascore library
-    >>> pa = dascore.get_example_patch() # generate example patch
+    >>> import dascore as dc  # import dascore library
+    >>> pa = dc.get_example_patch()  # generate example patch

tests/test_transform/test_fourier.py (5)

9-16: Prefer unit constant import over ad-hoc “seconds” alias

Import second once at module scope and use it consistently. This removes the custom seconds = get_quantity("seconds").

Apply:

 from __future__ import annotations
@@
 import dascore as dc
-from dascore.exceptions import PatchError
-from dascore.transform.fourier import dft, idft
-from dascore.units import get_quantity
+from dascore.exceptions import PatchError
+from dascore.transform.fourier import dft, idft
+from dascore.units import get_quantity, second
@@
-F_0 = 2
-seconds = get_quantity("seconds")
+F_0 = 2

And update usage below (see next hunk).

---

`66-71`: **Use the imported unit constant**

Replace `seconds` variable with `second`.

Apply:

```diff
 def chirp_stft_patch(chirp_patch):
     """Perform sensible stft on patch."""
-    out = chirp_patch.stft(time=0.5 * seconds)
+    out = chirp_patch.stft(time=0.5 * second)
     return out.update_attrs(history=[])

---

300-307: Minor consistency: compare to local variable

Use patch2 for the final equality check (already unpacked above).

Apply:

-        assert patch1.equals(chirp_patch, close=True)
+        assert patch1.equals(patch2, close=True)

---

308-314: Avoid function-local imports in tests

Reuse module-level imports for consistency and speed.

Apply:

-    def test_round_trip_2(self):
+    def test_round_trip_2(self):
         """Another round trip test from the doctests."""
-        import dascore as dc
-        from dascore.units import second
-
         patch = dc.get_example_patch("chirp")
         # Simple stft with 10 second window and 4 seconds overlap
         pa1 = patch.stft(time=10 * second, overlap=4 * second)

---

318-330: Add a cleanup assertion after istft

Validate that private STFT metadata/coords don’t leak after inverse.

Apply (adds a new test):

         assert pa2.equals(patch, close=True)
 
+    def test_no_private_stft_metadata_after_inverse(self, chirp_stft_patch):
+        """_stft* attrs/coords should be removed after istft."""
+        pa2 = chirp_stft_patch.istft()
+        assert not any(k.startswith("_stft") for k in dict(pa2.attrs).keys())
+        assert "_stft_window" not in pa2.coords.coord_map

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

tests/test_transform/test_fourier.py (6)

285-290: Fix typo: “insure” → “ensure”.

-    def test_type(self, chirp_stft_patch, chirp_patch):
-        """Simply insure the correct type was returned."""
+    def test_type(self, chirp_stft_patch, chirp_patch):
+        """Simply ensure the correct type was returned."""

---

291-296: Add a negative test for mismatched window length vs time samples.
Currently, passing an ndarray window whose length ≠ window_samples could misconfigure mfft/window pairing. Add a test that asserts a clear error or automatic alignment.

Would you like me to draft the test?

---

301-305: Consider hoisting this fixture to module scope.
Slightly improves discoverability/reuse; behavior remains identical.

-    @pytest.fixture(scope="class")
-    def chirp_round_tripped(self, chirp_stft_patch):
-        """Round trip patch through stft."""
-        return chirp_stft_patch.istft()
+@pytest.fixture(scope="session")
+def chirp_round_tripped(chirp_stft_patch):
+    """Round trip patch through stft."""
+    return chirp_stft_patch.istft()

---

314-324: Remove redundant inner imports; use module-level imports.
Minor cleanup.

-        import dascore as dc
-        from dascore.units import second
-
-        patch = dc.get_example_patch("chirp")
+        patch = dc.get_example_patch("chirp")
         # Simple stft with 10 second window and 4 seconds overlap
-        pa1 = patch.stft(time=10 * second, overlap=4 * second)
+        pa1 = patch.stft(time=10 * dc.get_unit("s"), overlap=4 * dc.get_unit("s"))

Alternatively, import second once at module top.

---

325-330: Docstring typo: should be “hasn’t undergone stft”.

-        """A patch that hasn't undergone istft can't be used."""
+        """A patch that hasn't undergone stft can't be used."""

Also, good job asserting the error message substring.

---

314-324: Optional: Assert STFT/ISTFT metadata cleanup after round-trip.
After istft, _stft* attrs should be gone. Add a quick check to prevent regressions.

         pa2 = pa1.istft()
-        assert pa2.equals(patch, close=True)
+        assert pa2.equals(patch, close=True)
+        assert not any(k.startswith("_stft") for k in dict(pa2.attrs))

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

dascore/transform/fourier.py (2)

160-161: Fix incorrect cross-reference in See Also section.

The second reference should be to stft, not idft again.

-    - [idft](`dascore.transform.fourier.idft`)
-    - [stft](`dascore.transform.fourier.stft`)
+    - [idft](`dascore.transform.fourier.idft`)
+    - [stft](`dascore.transform.fourier.stft`)

---

430-434: Add validation for sampling rate calculation.

When the coordinate step is zero or None, the sampling rate calculation will fail. Consider adding validation.

     coord = patch.get_coord(dim, require_evenly_sampled=True)
     window_samples = coord.get_sample_count(val, samples=samples, enforce_lt_coord=True)
+    if coord.step is None or coord.step == 0:
+        msg = f"Coordinate {dim} must be evenly sampled for STFT"
+        raise PatchError(msg)
     sampling_rate = 1 / dc.to_float(coord.step)

dascore/transform/fourier.py (1)

310-311: Fix cross-reference consistency in idft See Also section.

Update the second reference to maintain consistency with the dft function's See Also section.

-    - [dft](`dascore.transform.fourier.dft`)
-    - [istft](`dascore.transform.fourier.istft`)
+    - [dft](`dascore.transform.fourier.dft`)
+    - [istft](`dascore.transform.fourier.istft`)

.github/actions/mamba-install-dascore/action.yml (1)

45-45: post-cleanup: "shell-init" is valid (and is the default); optional to keep.

Docs list none | shell-init | environment | all, with shell-init as the default. Keeping this is fine for GitHub-hosted runners; if you move to self-hosted and want stronger isolation, consider environment or all. (github.com)

Apply if you prefer relying on defaults:

-        post-cleanup: "shell-init"
+        # post-cleanup: default is "shell-init"; override only if you need stronger cleanup

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

dascore/utils/time.py (1)

383-401: Fix isinstance() with unions to support Python ≤3.10.

Using A | B in isinstance raises TypeError on py310. Replace with tuples.

Apply:

-    if isinstance(obj, numpy_dtype | pandas_dtype):
+    if isinstance(obj, (numpy_dtype, pandas_dtype)):
@@
-    if isinstance(obj, (np.dtype | pd.api.extensions.ExtensionDtype)):
+    if isinstance(obj, (np.dtype, pd.api.extensions.ExtensionDtype)):
@@
-    if isinstance(obj, np.ndarray | list | tuple):
+    if isinstance(obj, (np.ndarray, list, tuple)):

dascore/core/attrs.py (1)

250-256: Type the signature and simplify via model_dump(exclude=...)

Leverage pydantic’s exclude to avoid building intermediate sets/dicts and add explicit types/return for clarity.

-    def drop(self, *args):
-        """Drop specific keys if they exist."""
-        contents = dict(self)
-        ok_to_keep = set(contents) - set(args)
-        out = {i: v for i, v in contents.items() if i in ok_to_keep}
-        return self.__class__(**out)
+    def drop(self, *keys: str) -> Self:
+        """Return a new PatchAttrs with the given top-level keys removed (unknown keys are ignored)."""
+        drop_keys = frozenset(keys)
+        new = self.model_dump(exclude=drop_keys)
+        return self.__class__(**new)

dascore/utils/time.py (1)

405-408: Docstring mismatch (datetime vs timedelta).

Update to reflect datetime64.

-def is_datetime64(obj):
-    """Determine if an object represents a timedelta64 dtype or value(s)."""
+def is_datetime64(obj):
+    """Determine if an object represents a datetime64 dtype or value(s)."""

dascore/transform/fourier.py (2)

448-451: Validate fft_mode choice for complex input across SciPy versions.

If "centered" isn’t supported for your SciPy target, prefer "twosided" and apply fftshift yourself, or document the minimum required SciPy.

---

430-447: Guard hop/window edge cases.

coord.get_sample_count enforces overlap < window; consider asserting hop > 0 to fail fast on pathological inputs.

 hop = window_samples - overlap
+assert hop > 0, "STFT hop must be > 0."

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: d-chambers
PR: DASDAE/dascore#523
File: dascore/transform/fourier.py:433-434
Timestamp: 2025-09-02T16:22:22.829Z
Learning: In dascore STFT implementation, use `require_evenly_sampled=True` on `get_coord` calls instead of manual validation for coord.step to ensure sampling rate calculations are safe.

dascore/transform/fourier.py (3)

376-376: Decorator likely misapplied; use @patch_function() (call it).

Without calling, stft may not be registered as a Patch method.

-@patch_function
+@patch_function()
 def stft(

---

531-534: Expose istft as a Patch method.

Add the patch decorator so patch.istft() works.

- def istft(
+@patch_function()
+def istft(
     patch,
 ):

---

499-515: Fix _get_istft_coord to return a Coord (not a (None, coord) tuple) and reassign with the correct key.

Current code stores and re-injects the tuple; this breaks len(coord) and shape checks later, and causes the scale/divide in istft to fail.

-    coord_map.pop("_stft_window")
-    time = coord_map.pop("_stft_old_coord")
-    coord_map.pop(coords.dims[frequency_axis])
-    coord_map[dims[time_axis]] = time
+    coord_map.pop("_stft_window")
+    # Unwrap the stored original coordinate.
+    orig_time_coord = coord_map.pop("_stft_old_coord")[1]
+    freq_name = dims[frequency_axis]
+    time_name = dims[time_axis]
+    coord_map.pop(freq_name)
+    # Reassign the original time coord under the proper key.
+    coord_map[time_name] = (time_name, orig_time_coord)
@@
-    new_dims[frequency_axis] = dims[time_axis]
-    new_dims.pop(time_axis)
-    return get_coord_manager(coords=coord_map, dims=new_dims), time
+    new_dims[frequency_axis] = dims[time_axis]
+    new_dims.pop(time_axis)
+    return get_coord_manager(coords=coord_map, dims=new_dims), orig_time_coord

dascore/transform/spectro.py (1)

61-66: Place @deprecate outermost to ensure deprecation is visible to users and tooling.

With the current order, patch_function() wraps the deprecated function, which can hide the typing-level deprecation and doc updates. Make deprecate the top decorator.

-@patch_function()
-@deprecate(
-    info="Use Patch.stft() instead.",
-    since="0.1.11",
-    removed_in="0.2.0",
-)
+@deprecate(
+    info="Use Patch.stft() instead.",
+    since="0.1.11",
+    removed_in="0.2.0",
+)
+@patch_function()
 def spectrogram(patch: PatchType, dim: "str" = "time", **kwargs) -> PatchType:

tests/test_transform/test_fourier.py (2)

12-16: Avoid two “second(s)” aliases; use the canonical second.

Drop the custom seconds alias and use the imported second for clarity.

-from dascore.units import get_quantity, second
+from dascore.units import get_quantity, second
@@
-seconds = get_quantity("seconds")
@@
-    out = chirp_patch.stft(time=0.5 * seconds)
+    out = chirp_patch.stft(time=0.5 * second)

Also applies to: 67-71

---

306-327: Make the DFT<>STFT equivalence assertion robust to zero magnitudes.

Ratio can blow up on zeros. Compare magnitudes directly with tolerances.

-        factor = np.abs(ar1) / np.abs(ar2)
-        assert np.allclose(factor, 1.0)
+        assert np.allclose(np.abs(ar1), np.abs(ar2), rtol=1e-6, atol=1e-12)

dascore/transform/fourier.py (2)

426-431: Fix minor docstring typos.

Small polish for user-facing docs.

-    - The output is scalled the same as [Patch.dft](`dascore.Patch.dft`).
-    - For a given sliding window, Parseval's thereom doesn't hold exactly
-      unless a boxcare window is used.
+    - The output is scaled the same as [Patch.dft](`dascore.Patch.dft`).
+    - For a given sliding window, Parseval's theorem doesn't hold exactly
+      unless a boxcar window is used.

---

535-541: Tighten ISTFT docstring phrasing.

-    Inverse a short-time fourier transform.
+    Inverse the short-time Fourier transform.
@@
-        A patch return from [stft](`dascore.transform.fourier.stft`).
+        A Patch returned from [stft](`dascore.transform.fourier.stft`).

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: d-chambers
PR: DASDAE/dascore#523
File: dascore/transform/fourier.py:433-434
Timestamp: 2025-09-02T16:22:22.829Z
Learning: In dascore STFT implementation, use `require_evenly_sampled=True` on `get_coord` calls instead of manual validation for coord.step to ensure sampling rate calculations are safe.

dascore/transform/fourier.py (3)

439-444: Good: robust coord handling for STFT.

Using get_dim_axis_value + get_coord(require_evenly_sampled=True) + get_sample_count(enforce_lt_coord=True) is the right pattern here.

---

376-384: Decorator bug: call patch_function().

Without calling, the decorator likely won’t register stft as a Patch method.

-@patch_function
+@patch_function()
 def stft(

---

501-517: Bug: _get_istft_coord returns a (None, coord) tuple; must unwrap.

Returning/assigning the tuple breaks coord.step and len(coord) downstream.

     coord_map = dict(coords.coord_map)
     coord_map.pop("_stft_window")
-    time = coord_map.pop("_stft_old_coord")
-    coord_map.pop(coords.dims[frequency_axis])
-    coord_map[dims[time_axis]] = time
+    # Unwrap stored original coordinate.
+    orig_time_coord = coord_map.pop("_stft_old_coord")[1]
+    freq_name = dims[frequency_axis]
+    time_name = dims[time_axis]
+    coord_map.pop(freq_name)
+    coord_map[time_name] = orig_time_coord
@@
-    new_dims = list(dims)
-    new_dims[frequency_axis] = dims[time_axis]
-    new_dims.pop(time_axis)
-    return get_coord_manager(coords=coord_map, dims=new_dims), time
+    new_dims = list(dims)
+    new_dims[frequency_axis] = time_name
+    new_dims.pop(time_axis)
+    return get_coord_manager(coords=coord_map, dims=new_dims), orig_time_coord

dascore/transform/fourier.py (3)

430-433: Fix typos in Notes.

Small doc fixes.

-    - The output is scalled the same as [Patch.dft](`dascore.Patch.dft`).
-    - For a given sliding window, Parseval's thereom doesn't hold exactly
-      unless a boxcare window is used.
+    - The output is scaled the same as [Patch.dft](`dascore.Patch.dft`).
+    - For a given sliding window, Parseval's theorem doesn't hold exactly
+      unless a boxcar window is used.

---

442-444: Minor: simplify float conversions.

No need to convert twice.

-    step = dc.to_float(coord.step)
-    sampling_rate = 1 / dc.to_float(step)
+    step = to_float(coord.step)
+    sampling_rate = 1 / step

---

364-371: Optional: use get_coord_tuple_map() for consistency with DFT path.

You’re mixing BaseCoord values with (None, obj) tuples in coord_map. Prefer building from get_coord_tuple_map() for uniformity.

-    coord_map = dict(patch.coords.coord_map)
+    coord_map = patch.coords.get_coord_tuple_map()
@@
-            "_stft_window": (None, window),
-            "_stft_old_coord": (None, patch.get_coord(dim)),
+            "_stft_window": (None, window),
+            "_stft_old_coord": (None, patch.get_coord(dim)),

docs/tutorial/transformations.qmd (2)

51-54: Terminology: “Short-Time Fourier Transform.”

Use the hyphenated form and correct term.

-# Short Time Fourier Transform
+# Short-Time Fourier Transform
@@
-Related to the Discrete Fourier Transform, the [Short Term Fourier Transform]
+Related to the Discrete Fourier Transform, the [Short-Time Fourier Transform]

---

73-78: Strengthen example: verify round-trip.

Add an assertion to demonstrate invertibility in the tutorial.

 inverse =  transformed.istft()
 
 transformed.abs().select(distance=0, samples=True).squeeze().viz.waterfall()
+assert inverse.equals(patch, close=True)

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: d-chambers
PR: DASDAE/dascore#523
File: dascore/transform/fourier.py:433-434
Timestamp: 2025-09-02T16:22:22.829Z
Learning: In dascore STFT implementation, use `require_evenly_sampled=True` on `get_coord` calls instead of manual validation for coord.step to ensure sampling rate calculations are safe.

Learnt from: d-chambers
PR: DASDAE/dascore#523
File: dascore/transform/fourier.py:433-434
Timestamp: 2025-09-02T16:22:22.829Z
Learning: In dascore STFT implementation, use `require_evenly_sampled=True` on `get_coord` calls instead of manual validation for coord.step to ensure sampling rate calculations are safe.

dascore/transform/fourier.py (3)

376-376: Use the callable decorator form for consistency.

Elsewhere in this module decorators are used as @patch_function(). Do the same here to avoid subtle decorator semantics issues.

-@patch_function
+@patch_function()
 def stft(

---

448-452: Validate custom window length matches window_samples.

Passing a mismatched ndarray silently changes frequency resolution (zero-padding effect). Fail fast.

     if isinstance(taper_window, ndarray):
-        window = taper_window
+        window = taper_window
+        if len(window) != window_samples:
+            raise PatchError(
+                f"Window length ({len(window)}) must equal window_samples ({window_samples})."
+            )
     else:
         window = get_window(taper_window, window_samples, fftbins=False)

---

536-539: Expose istft as a Patch method and add typing.

Match dft/idft by using the callable decorator and typing the parameter.

-@patch_function
-def istft(
-    patch,
-):
+@patch_function()
+def istft(
+    patch: PatchType,
+) -> PatchType:

environment.yml (1)

9-9: Consider aligning constraint style for pydantic.

Use >=2.1 rather than >2.1 to avoid excluding the exact 2.1.* series.

-  - pydantic>2.1
+  - pydantic>=2.1

dascore/transform/fourier.py (4)

406-409: Remove undocumented psd mention or implement it.

psd is documented but not in the signature or implementation.

-    psd
-        If True, return the power spectral density (PSD).

Or add psd: bool = False and implement PSD scaling.

---

504-520: Return tuple ok; prefer tuple for dims and clarify variable.

get_coord_manager accepts tuple dims; return a tuple for immutability and readability.

-    new_dims = list(dims)
+    new_dims = list(dims)
@@
-    return get_coord_manager(coords=coord_map, dims=new_dims), time
+    return get_coord_manager(coords=coord_map, dims=tuple(new_dims)), time

---

559-567: Clearer error when inversion isn’t possible.

Include reason (detrended vs non-invertible window/fft_mode) to guide users.

-    if detrended or not stft.invertible:
-        msg = f"Inverse stft not possible for patch {patch}."
+    if detrended or not stft.invertible:
+        why = "detrended" if detrended else "non-invertible STFT settings"
+        msg = f"Inverse STFT not possible ({why})."
         raise PatchError(msg)

---

569-577: Trim axis selection is subtle—looks correct; add a brief comment.

Slicing along frequency_axis is correct because that axis becomes the time axis after ISTFT. Add a one-liner to prevent future regressions.

-    index = broadcast_for_index(
+    # Note: after ISTFT, `frequency_axis` now indexes the restored time axis.
+    index = broadcast_for_index(
         data_untrimmed.ndim, frequency_axis, slice(0, len(coord))
     )

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: d-chambers
PR: DASDAE/dascore#523
File: dascore/transform/fourier.py:433-434
Timestamp: 2025-09-02T16:22:22.829Z
Learning: In dascore STFT implementation, use `require_evenly_sampled=True` on `get_coord` calls instead of manual validation for coord.step to ensure sampling rate calculations are safe.

Learnt from: d-chambers
PR: DASDAE/dascore#523
File: dascore/transform/fourier.py:433-434
Timestamp: 2025-09-02T16:22:22.829Z
Learning: In dascore STFT implementation, use `require_evenly_sampled=True` on `get_coord` calls instead of manual validation for coord.step to ensure sampling rate calculations are safe.

dascore/transform/fourier.py (3)

376-377: Decorator must be called: use @patch_function().

This matches other transform APIs and avoids decorator-factory surprises.

-@patch_function
+@patch_function()
 def stft(

---

449-453: Validate custom window length equals window_samples.

Mismatched ndarray windows can error or yield incorrect transforms.

 if isinstance(taper_window, ndarray):
-    window = taper_window
+    window = taper_window
+    if len(window) != window_samples:
+        raise PatchError(
+            f"Window length ({len(window)}) must equal window_samples ({window_samples})."
+        )
 else:
     window = get_window(taper_window, window_samples, fftbins=False)

---

505-521: Fix: _get_istft_coord returns a (None, coord) tuple; must unwrap Coord.

This breaks coord.step/len in istft; also keep names explicit.

-    coord_map = dict(coords.coord_map)
-    coord_map.pop("_stft_window")
-    time = coord_map.pop("_stft_old_coord")
-    coord_map.pop(coords.dims[frequency_axis])
-    coord_map[dims[time_axis]] = time
+    coord_map = dict(coords.coord_map)
+    # Remove stored window; unwrap and restore original time coord.
+    coord_map.pop("_stft_window")
+    orig_time_coord = coord_map.pop("_stft_old_coord")[1]
+    freq_name = dims[frequency_axis]
+    time_name = dims[time_axis]
+    coord_map.pop(freq_name)
+    coord_map[time_name] = orig_time_coord
@@
-    new_dims = list(dims)
-    new_dims[frequency_axis] = dims[time_axis]
-    new_dims.pop(time_axis)
-    return get_coord_manager(coords=coord_map, dims=new_dims), time
+    new_dims = list(dims)
+    new_dims[frequency_axis] = time_name
+    new_dims.pop(time_axis)
+    return get_coord_manager(coords=coord_map, dims=new_dims), orig_time_coord

dascore/transform/fourier.py (6)

341-374: Prefer invert_quantity for unit inversion; keep style consistent.

Use the existing helper for clarity.

-    new_units = None
-    if coord.units is not None:
-        new_units = 1 / dc.get_quantity(coord.units)
+    new_units = invert_quantity(coord.units) if coord.units is not None else None

---

406-408: Doc mismatch: remove undocumented psd parameter.

psd isn’t in the signature or code; drop from docs (or implement).

-    psd
-        If True, return the power spectral density (PSD).

---

430-436: Doc typos.

“zero pading” → “zero padding”; minor clarity tweaks.

-    - If an array is passed for taper_window that has a different length
-      than specified in kwargs, artificial enriching of frequency resolution
-      (equivalent to zero pading in time domain) can occur.
+    - If an array is passed for `taper_window` with a length different from
+      `window_samples`, it artificially enriches frequency resolution
+      (equivalent to zero padding in the time domain).

---

446-447: Tiny cleanup: avoid double conversion.

You already converted step to float.

-step = dc.to_float(coord.step)
-sampling_rate = 1 / dc.to_float(step)
+step = dc.to_float(coord.step)
+sampling_rate = 1.0 / step

---

523-535: Harden reconstruction: validate required attrs/coords exist.

Fail fast with a helpful message if any STFT metadata is missing.

 def _get_short_time_fft(patch) -> ShortTimeFFT:
     """Reconstruct the short time fft from the attrs/coords in patch."""
-    sr = patch.attrs.get("_stft_sampling_rate")
+    sr = patch.attrs.get("_stft_sampling_rate")
+    win_coord = patch.get_coord("_stft_window", require_sorted=False)
+    hop = patch.attrs.get("_stft_hop")
+    fft_mode = patch.attrs.get("_stft_fft_mode")
+    mfft = patch.attrs.get("_stft_mfft")
+    if any(x is None for x in (sr, hop, fft_mode, mfft)) or win_coord is None:
+        raise PatchError("Patch is missing STFT metadata; cannot reconstruct transformer.")
     # Recreate STFFT class based on saved coords/attrs.
     stft = ShortTimeFFT(
-        win=patch.get_coord("_stft_window").values,
-        hop=patch.attrs.get("_stft_hop"),
-        fs=sr,
-        fft_mode=patch.attrs.get("_stft_fft_mode"),
-        mfft=patch.attrs.get("_stft_mfft"),
+        win=win_coord.values,
+        hop=hop,
+        fs=sr,
+        fft_mode=fft_mode,
+        mfft=mfft,
     )
     return stft

---

537-541: Add typing to istft; minor grammar nit in doc.

Keep parity with other APIs.

-@patch_function()
-def istft(
-    patch,
-):
+@patch_function()
+def istft(patch: PatchType) -> PatchType:
@@
-    Inverse a short-time fourier transform.
+    Invert a short-time fourier transform.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: d-chambers
PR: DASDAE/dascore#523
File: dascore/transform/fourier.py:433-434
Timestamp: 2025-09-02T16:22:22.829Z
Learning: In dascore STFT implementation, use `require_evenly_sampled=True` on `get_coord` calls instead of manual validation for coord.step to ensure sampling rate calculations are safe.

Learnt from: d-chambers
PR: DASDAE/dascore#523
File: dascore/transform/fourier.py:433-434
Timestamp: 2025-09-02T16:22:22.829Z
Learning: In dascore STFT implementation, use `require_evenly_sampled=True` on `get_coord` calls instead of manual validation for coord.step to ensure sampling rate calculations are safe.

dascore/transform/fourier.py (3)

373-381: Use @patch_function() (callable form) for stft.

Other transforms use the callable form; this keeps decorator semantics consistent.

-@patch_function
+@patch_function()
 def stft(

---

444-451: Validate custom window length equals window_samples.

Mismatched ndarray windows will cause incorrect STFT geometry or errors later; fail fast.

     if isinstance(taper_window, ndarray):
-        window = taper_window
+        window = taper_window
+        if len(window) != window_samples:
+            raise PatchError(
+                f"Window length ({len(window)}) must equal window_samples ({window_samples})."
+            )

---

500-516: _get_istft_coord returns tuple instead of Coord; breaks istft.

You’re returning (None, coord) and assigning it back as-is, which propagates a tuple and causes coord.step access to fail. Unwrap and reassign with the correct key; also return the Coord.

-    coord_map.pop("_stft_window")
-    time = coord_map.pop("_stft_old_coord")
-    coord_map.pop(coords.dims[frequency_axis])
-    coord_map[dims[time_axis]] = time
+    coord_map.pop("_stft_window")
+    # Unwrap the stored original coordinate.
+    orig_time_coord = coord_map.pop("_stft_old_coord")[1]
+    freq_name = dims[frequency_axis]
+    time_name = dims[time_axis]
+    coord_map.pop(freq_name)
+    coord_map[time_name] = orig_time_coord
@@
-    new_dims = list(dims)
-    new_dims[frequency_axis] = dims[time_axis]
-    new_dims.pop(time_axis)
-    return get_coord_manager(coords=coord_map, dims=tuple(new_dims)), time
+    new_dims = list(dims)
+    new_dims[frequency_axis] = time_name
+    new_dims.pop(time_axis)
+    return get_coord_manager(coords=coord_map, dims=tuple(new_dims)), orig_time_coord

dascore/transform/fourier.py (2)

441-443: Use local to_float import and guard against negative step.

Minor consistency and robustness: prefer to_float over dc.to_float; also ensure fs is positive in case of reverse-sorted coords.

-    step = dc.to_float(coord.step)
-    sampling_rate = 1 / step
+    step = to_float(coord.step)
+    sampling_rate = 1 / abs(step)

---

532-536: Type annotate istft signature and return type.

Align with stft/dft/idft for clarity and tooling.

-@patch_function()
-def istft(
-    patch,
-):
+@patch_function()
+def istft(patch: PatchType) -> PatchType:

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: d-chambers
PR: DASDAE/dascore#523
File: dascore/transform/fourier.py:433-434
Timestamp: 2025-09-02T16:22:22.829Z
Learning: In dascore STFT implementation, use `require_evenly_sampled=True` on `get_coord` calls instead of manual validation for coord.step to ensure sampling rate calculations are safe.

Learnt from: d-chambers
PR: DASDAE/dascore#523
File: dascore/transform/fourier.py:433-434
Timestamp: 2025-09-02T16:22:22.829Z
Learning: In dascore STFT implementation, use `require_evenly_sampled=True` on `get_coord` calls instead of manual validation for coord.step to ensure sampling rate calculations are safe.