test coverage for filters#229
Conversation
| raise TypeError("This filter only accepts xr.DataArray or xr.Dataset") | ||
|
|
||
| if not function(sample): | ||
| if not drop: |
There was a problem hiding this comment.
From the doc string for DropValue:
Filter to drop data containing more than a given percentage of a value.
all the DropValue filters calculate whether % of elemns matching value >= the percentage, and then not's them here, which does the opposite of what the doc string says. @tennlee should I follow what the doc string says or leave the logic as is?
There was a problem hiding this comment.
The top-level filter class says "filter should sairse a PipelineFilterException if invalid. So I think the not is unwanted? The test for me is that a pipelinefilterexception should be raised if data with a high proportion of nans (or whatever query value) is supplied.
Pull Request Test Coverage Report for Build 20294261772Details
💛 - Coveralls |
152094d to
0b43a9f
Compare
|
covered dask case so pr is ready to review. |
|
not sure why ci has suddenly started failing at install depends step... |
|
Sorry for the delay here ... a few things at work meant a building of reviews, and I had some difficult refactoring to do on "scores". I'll get to this next week, maybe earlier. |
|
I note the unit tests are failing due to the dependencies now exceeding the disk space allowed by the CI/CD system. I am trying to push up changes to bring the CI/CD setup back within the tolerances set by Github. |
|
@edoyango okay, so I think the CI/CD is doing its job again. However, surprisingly, one of the tests i failing on Python 3.13 specifically, which is unexpectedly. Let me know if you can take a look. If I can figure it out, I'll happily push up a fix, but the answer is not springing to mind. |
|
@edoyango I'm going to merge this to get the obvious benefits from improved coverage and getting the CI/CD passing again. However I've raised a new issue around the point you identified about the directionality of the logic in the filter. If you have time to address it, please do. |
This adds test coverage for filtering for numpy, dask, xarray. Contains a few tests unrelated to filtering. Some of the xarray filtering seemed improperly implemented (looks like it was copied from numpy), so I added some proper implementations.
WIP.