Hey @d-chambers, please review and let me know what you think at your convenience. Also, I wonder how I can adjust the _create_size_fw_and_axes(patch, kwargs) function so it can accept specified units as well (please see two commented tests in tests/test_proc/test_filter.py) Thanks!

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.84%. Comparing base (e6ddf30) to head (a82f18f).
Report is 1 commits behind head on master.

@@           Coverage Diff           @@
##           master     #448   +/-   ##
=======================================
  Coverage   99.84%   99.84%           
=======================================
  Files         114      114           
  Lines        9184     9203   +19     
=======================================
+ Hits         9170     9189   +19     
  Misses         14       14           

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

Also, I wonder how I can adjust the _create_size_fw_and_axes(patch, kwargs) function so it can accept specified units as well (please see two commented tests in tests/test_proc/test_filter.py) Thanks!

Probably the easiest thing to do for the units is to pull get_inverted_quant from being a nested function to being on the dascore.units level. Then you can do something like this (but I haven't tested it yet):

from dascore.units import get_inverted_quant
from dascore.utils.time import to_float

size, axes = _create_size_fw_and_axes(patch, kwargs)
data = patch.data
for coord_name, info in get_multiple_dim_value_from_kwargs(patch, kwargs).items():
    dim, ax, value = info['dim'], info['axis'], info['value']
    coord = patch.get_coord(coord_name, required_evenly_sampled=True)  # will also raise if not evenly sampled.

    # Invert units if needed
    inverted_units = False
    if isinstance(value, dc.units.Quantity) and coord.units is not None:
        value, inverted_units = get_inverted_quant(value, coord.units)
    
    # Check valid parameters
    sr, value = to_float(coord), to_float(value)
    nyquist = 0.5 * sr 
    if w0 > nyquist:
        msg = f"possible filter values are in [0, {nyquist}] you passed {w0}"
        raise FilterValueError(msg)

    b, a = iirnotch(value, Q=q, fs=sr)
    data = filtfilt(b, a, data, axis=ax)

note: the imports should be at the top of the module, just put them here for reference.

        value, inverted_units = get_inverted_quant(value, coord.units)

Thanks for this explanation, Derrick! I'm unsure why the inverted_units is being reported False when the get_inverted_quant actually has inverted the unit! Do we need to fix this inside the get_inverted_quant function?

Do we need to fix this inside the get_inverted_quant function?

So the function returns the correctly inverted quantity but just the boolean is wrong? Yes, I suppose we should fix it, hopefully it hasn't caused issues elsewhere.

Sorry about the linter failing. It has to do with newer versions of to pip I think. I will push a fix in a few minutes.

So the function returns the correctly inverted quantity but just the boolean is wrong? Yes, I suppose we should fix it, hopefully it hasn't caused issues elsewhere.

Actually, looking closer I think it is correct, just a bit confusing. With filtering we do expect the dimensionality of inputs to the be inverse of the coordinate units. For example, for the time dimension (seconds) we expect the filter parameters to be the multiplicative inverse dimensionality (1/seconds, Hz). So the second parameter returned from this function just indicates if the dimensionality of the filter argument were already the inverse dimensionality. Its probably safe to just ignore the second output parameter.

closes #443.