Add filter parent class and use it for WienerFilter
This is the first merge request in a potential series of two with the following goals:
- To implement a common parent class for all present and future filter implementations
- Common interface for all filters
- Minimizes duplicate code
- Implement a shared test suite for filter implementations
- Add a least mean squares (LMS) filter implementation that uses the new parent class
This first MR addresses the implementation of shared parent classes.
Commit concept:
-
Implement filter parent class and adjust WF implementation to use it (without touching the tests or ipython notebooks to ensure nothing relevant is changed) -
Code cleanup - The original WF implementation uses a lot of python loops. This commit replaces some of them with faster
numpyequivalents. - The original WF implementation contains an MSE function that to my understanding actually contains a definition of the square root of MSE (so effectively RMS). I've removed the square root. I only noticed this function being used as an input for a minimizer, so the square root probably had no significant effect.
- The original WF implementation uses a lot of python loops. This commit replaces some of them with faster
-
Build a shared test suite for filters and use it for the WF
In a separate merge request:
- Implement LMS filter
Implementation of a common interface class
My goal for the first commit is to maintain as much of the user interface of the existing WienerFitler implementation. This allows testing against the current test suite.
There were however a few things that I have intentionally changed or not included in the shared interface:
- Multiprocessing and zero padding behavior can be set both for the whole class and for an individual apply call for
WienerFilter. Having this setting for the whole class complicates the code and feels unnecessary to me. - I've switched to using
dataclassesto define which paramters of the filter classes will be exported in the JSON dumps. This works with inheritance and collects that information in a defined place at the top of the class. It also enforces one common naming scheme for member variables and the JSON dumps. - I don't feel like some aspects of the file handling during saving and loading are very intuitive
-
save()contains a line that replaces backslashes in the path with slashes for windows. This creates a potenially unexpected different behavior to all other python path handling.path = path.replace("\\", "/") # get rid of Windows-only path separator if any
- I don't see how raising a
ValueErroris more descriptive then just letting theopen()raise the mathcing exception (e.g.FileNotFoundError)
-
- Add an exception for calling apply() on an uninitialized class to give understandable feedback (after running into that issue myself while debugging issues with the documentation ipython notebook).
- Moved the
indexparameter toWienerFitler.apply()to the end, because this is not a parameter that all filters will need.
Implement shared test suite functionality for filters
A new parent class TestFilter was added that contains test cases that all filter implementations shall meet.
Specific test cases for WienerFilter have been moved to the child class TestWienerFilter.
Old:
Required test coverage of 87.0% reached. Total coverage: 93.04%
===================================================================== 68 passed, 38 warnings in 46.95s ======================================================================
New:
Required test coverage of 87.0% reached. Total coverage: 93.04%
===================================================================== 67 passed, 36 warnings in 48.16s ======================================================================
Overall, the tests have gotten a tiny bit slower, which is because they are run for more scenarios now. Test coverage has not changed.
Open questions
- I don't understand why ignoring the linter error is okay in this line
Addresses #82