merging FZ and HI filter into one notebook and fix FZ filter code

OpenSenseAction / pypwsqc

Python package for quality control (QC) of data from personal weather stations (PWS)

https://pypwsqc.readthedocs.io

BSD 3-Clause "New" or "Revised" License

0 stars 3 forks source link

merging FZ and HI filter into one notebook and fix FZ filter code #31

Closed lepetersson closed 2 months ago

lepetersson commented 2 months ago

[UPDATE based on discussion online]

This PR should do the following:

[x] add notebook that applies FZ and HI filter in one go
[x] fix potential problems with FZ filter, discussed in #11 and #22
[x] expanding FZ filter to run for several stations
[x] switch FZ filter to requiring input as xr.DataArray and return xr.DataArray

lepetersson commented 2 months ago

[UPDATE based on discussion online]

This PR should do the following:

[x] add notebook that applies FZ and HI filter in one go

[x] fix potential problems with FZ filter, discussed in Improve FZ filter code #11 and FZ filter incorrectly assigns -1 #22

[x] expanding FZ filter to run for several stations

[x] switch FZ filter to requiring input as xr.DataArray and return xr.DataArray

Issues fixed 12 June

lepetersson commented 2 months ago

@cchwala I just pushed a version that passes the linting on my machine. Some checks were not successful (see below) but it seems to be issues with the other old notebooks that I just kept because... whatever. Should I remove them, only keep the notebook that is relevant now (merged_filters) and then push again?

cchwala commented 2 months ago

I suggest to remove faulty_zero_filter_LPW.ipynb since, as far as I understand, you do not want to have it in the final state of the PR that gets merged. It will be in the git history of the repository anyway, so that you can come back and even link to it if the need arises.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (e965b76) to head (6811d8f). Report is 6 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #31 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 2 2 Lines 23 32 +9 ========================================= + Hits 23 32 +9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cchwala commented 2 months ago

Thanks for working through the challenges of this PR! Looks good.

Just one minor thing which would make the notebook a lot nicer. Can you add some plots of the results of the flagging?

cchwala commented 2 months ago

Thanks for yet another updated. The notebooks now explain things much better.

There are some implementation details we can and should discuss later, e.g. as already done in #7, but this is now a very nice comprehensive showcase of the current functionality.

IMO this is ready to merge 🎉 🍾