Conversation
ryanhammonds
left a comment
There was a problem hiding this comment.
Looks good! I left a few comments/questions. I'm also about to push an accuracy test. Feel free to drop the test change if you think it's too much.
| # Get the set of frequency values that need to be interpolated | ||
| interp_mask = np.logical_and(freqs >= interp_range[0], freqs <= interp_range[1]) | ||
| interp_freqs = freqs[interp_mask] | ||
| # If given a list of interpolation zones, recurse to apply each one |
There was a problem hiding this comment.
After working on the #198, do you think it would be useful to generalize interpolate_spectrum to interpolate_spectra? I could see a case where one would want to remove line noise systematically across multiple spectra. Looping this function would be easy enough, however.
There was a problem hiding this comment.
This is a great idea! Just like the other util function, trim_spectrum, does, interpolate_spectrum (or renamed to spectra should totally support 2D arrays. If there is line noise in a dataset, it should be consistent across channels, and so applying this to a 2D collection of PSDs is if anything likely to be more common than to 1D.
Do you want to try out generalizing the interpolation to 2D? It can go into a new PR I think.
|
Thanks for the review @ryanhammonds - and the accuracy checks look great! I made some comments on specific points - I don't think there is anything needs doing to this PR now, so I'll merge, but a PR for extending interpolation would be great :). |
A common question is how to deal with line noise, and so I started collecting my responses to turn into a proper example, which I finally fixed up and is added here.
In the process, I ended up updating
interpolate_spectruma bit to support multiple interpolation zones.I think everything here should be relatively straight-forward - @ryanhammonds, it would be great if you could read through the example for wording quirks, and check that the code updates make sense to you. Thanks!