Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement relative altitude filter #42

Merged
merged 35 commits into from
Sep 6, 2024

Conversation

thorbjoernl
Copy link
Collaborator

Summary

Implement a relative altitude filter which filters stations based on the relative difference between station altitude and model altitude.

Related Issue

closes #39

@thorbjoernl thorbjoernl marked this pull request as ready for review September 5, 2024 08:42
@thorbjoernl thorbjoernl added this to the m2024-09 milestone Sep 5, 2024
Copy link
Member

@heikoklein heikoklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The specification was a bit unclear. We want to exclude stations which are to far off the gridded topography. To far off is absolute, e.g. 500m independed of topography being at 0m or at 2000m.

setup.cfg Outdated Show resolved Hide resolved
src/pyaro/timeseries/Filter.py Outdated Show resolved Hide resolved
src/pyaro/timeseries/Filter.py Show resolved Hide resolved
src/pyaro/timeseries/Filter.py Show resolved Hide resolved
src/pyaro/timeseries/Filter.py Outdated Show resolved Hide resolved
src/pyaro/timeseries/Filter.py Outdated Show resolved Hide resolved
src/pyaro/timeseries/Filter.py Outdated Show resolved Hide resolved
src/pyaro/timeseries/Filter.py Outdated Show resolved Hide resolved
@thorbjoernl
Copy link
Collaborator Author

CI fails with error if I don't include netcdf4 as an explicit dependency so I've added it back in for now:
image

Copy link
Member

@heikoklein heikoklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good. Please make dependencies optional (even if the break the altitude-filter if not installed).
Please use some time on performance-tuning.
The case for stations outside the grid is not documented/tested.

setup.cfg Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
src/pyaro/timeseries/Filter.py Show resolved Hide resolved
src/pyaro/timeseries/Filter.py Outdated Show resolved Hide resolved
@thorbjoernl thorbjoernl requested review from heikoklein and removed request for heikoklein September 6, 2024 06:10
@thorbjoernl thorbjoernl force-pushed the obs-mod-relative-altitude-filter branch from d58f22a to f79683c Compare September 6, 2024 06:41
Copy link
Member

@heikoklein heikoklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice. Thanks.

@heikoklein heikoklein merged commit 87ac570 into main Sep 6, 2024
1 check passed
@heikoklein heikoklein deleted the obs-mod-relative-altitude-filter branch September 6, 2024 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter for max difference between model-altitude and real altitude
2 participants