Skip to content

Series.spectral(): raise at the call site with actionable guidance when uneven - #707

Open
vaibhav8a wants to merge 1 commit into
LinkedEarth:masterfrom
vaibhav8a:fix/spectral-actionable-uneven-spacing
Open

vaibhav8a wants to merge 1 commit into
LinkedEarth:masterfrom
vaibhav8a:fix/spectral-actionable-uneven-spacing

Conversation

@vaibhav8a

Copy link
Copy Markdown

Fixes #698.

Problem

welch, mtm and periodogram refuse unevenly-spaced input from deep inside pyleoclim/utils/spectral.py:

raise ValueError('For the Welch method, data should be evenly spaced')

The message names neither the method the user actually called nor anything they can do about it. Series.wavelet() already does better for cwt, raising at the call site with the fix spelled out.

Change

A single guard at the top of Series.spectral(), mirroring Series.wavelet():

EVENLY_SPACED_SPECTRAL_METHODS = ['welch', 'mtm', 'periodogram', 'cwt']
...
if method in EVENLY_SPACED_SPECTRAL_METHODS and not self.is_evenly_spaced():
    raise ValueError(
        f"The chosen method is {method} but the series is unevenly spaced. "
        "You can either interpolate/bin (e.g. .interp(), .bin(), .gkernel()) "
        "or set method='lomb_scargle' or 'wwz', which handle unevenly-spaced series."
    )

lomb_scargle and wwz are deliberately excluded — both are built for uneven sampling. cwt is included so all four evenly-spaced-only methods report the same way; it previously surfaced from utils/wavelet.py.

On "move" vs. "add"

The issue says move the check up. I raised at the call site but left the utils/spectral.py guards in place, because removing them would silently drop validation for anyone calling specutils.welch() / mtm() / periodogram() directly — a public entry point. This is also exactly the split wavelet() uses today: an actionable raise in Series.wavelet() plus the low-level check still sitting at utils/wavelet.py:2531. Happy to strip the utils guards if you'd rather the check live in only one place.

Testing

Two new parametrized tests in TestUISeriesSpectral:

  • test_spectral_uneven_raises_with_guidance — for each of welch/mtm/periodogram/cwt, asserts the error names the method, mentions interp, and points at lomb_scargle.
  • test_spectral_uneven_allowed_methods — asserts lomb_scargle and wwz still run on the same uneven series, so the guard can't over-reach.

Confirmed they guard the change: with the series.py hunk stashed, all four uneven_raises cases fail.

pytest pyleoclim/tests/test_core_Series.py — 257 passed.

…en uneven

welch, mtm and periodogram refused unevenly-spaced input from inside
utils/spectral.py with a terse message ('For the Welch method, data should
be evenly spaced') that named neither the caller's method nor any fix.

Check at the Series.spectral() call site instead, the way Series.wavelet()
already does for cwt, and name the options: interp()/bin()/gkernel(), or
lomb_scargle/wwz which handle uneven sampling. The utils-layer guards stay
as the backstop for callers who reach specutils directly - the same split
wavelet() uses.

Fixes LinkedEarth#698

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Series.spectral(): unevenly-spaced error lacks actionable guidance

1 participant