Skip to content

Few methods silently accept invalid input. #215

Description

@sidxz

Nit picking here!, but it's a small robustness thing.
Some methods take dt_or_t and some handle NaNs, it's easy to try either on a method that doesn't support it.
Right now those cases don't complain, they just return inf or NaN.

Few that I found:

Time array into a fixed-step method (since t[0] == 0 it ends up dividing by zero):

importnumpyasnpfrompynumdiff.polynomial_fitimportsavgoldifffrompynumdiff.finite_differenceimportfinitediffdt=0.01; t=np.arange(0, 3, dt)
x=np.sin(3*t) +0.05*np.random.default_rng(0).standard_normal(len(t)); true_dxdt=3*np.cos(3*t)
_, d_ok=savgoldiff(x, dt, degree=2, window_size=9, smoothing_win=9)
_, d_bad=savgoldiff(x, t, degree=2, window_size=9, smoothing_win=9) # t by mistakeprint(np.abs(d_ok-true_dxdt).max(), np.abs(d_bad-true_dxdt).max()) # 1.51 infprint(np.abs(finitediff(x, t)[1] -true_dxdt).max()) # inf

spectraldiff does refuse, but with operands could not be broadcast together with shapes (1000,) (300,), which doesn't really say what went wrong.

NaN into a method :

frompynumdiff.basis_fitimportspectraldiffxn=x.copy(); xn[100] =np.nanprint(np.isnan(spectraldiff(xn, dt, high_freq_cutoff=0.2)[1]).sum()) # 300 of 300

kerneldiff does something similar (the NaNs spread through the convolution).
savgoldiff refuses via scipy's own check, which is fine.

This is actually very well handled in waveletdiff.

ValueError: `dt` must be a scalar. The DWT requires uniformly sampled data. For variable step sizes, use rbfdiff or splinediff instead.

Again, this one is a non blocker for me, nevertheless, reporting this for the better!

Review reference: openjournals/joss-reviews#11172

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions