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

Process for averaging over a polygon #315

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

huard
Copy link
Contributor

@huard huard commented Mar 5, 2020

Overview

This PR fixes #314

Changes:

  • Added AverageWFSPolygonProcess
  • Set aggregate=False in SubsetWFSPolygonProcess
  • Added smoke test.

Related Issue / Discussion

Additional Information

Links to other issues or sources.

@huard
Copy link
Contributor Author

huard commented Mar 5, 2020

This needs some more testing.

Copy link
Member

@nilshempelmann nilshempelmann left a comment

Choose a reason for hiding this comment

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

I can't test it, I have no WFS available which is required, if I got it right.
Cood looks good. Averaging is made in ocgis, make sure the averaging is done with wights depending on lat/lon since gridzells have different size: https://eggshell.readthedocs.io/en/latest/_modules/eggshell/nc/calculation.html#fieldmean

I ll dig deeper into it next week

@@ -22,7 +22,7 @@ replace = Version="{new_version}"
addopts =
--strict
--tb=native
tests/
Copy link
Contributor

Choose a reason for hiding this comment

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

Taking this out is causing Travis CI to find all written tests, including deprecated ones. Might want to place this back in.

@nilshempelmann
Copy link
Member

@huard Is there a public WFS to run this process if no local WFS is available, or could a polygon shapefile be used in stead, or the flyingpigeon intern polygon data (./flyingpigeon/data/shapefiles)

@huard
Copy link
Contributor Author

huard commented Mar 16, 2020

Started working on a shapefile polygon version.

Do you think shapefile and WFS inputs should be two independent processes, or the same process with different options to call it ?

We could use our own geoserver instance to test, but open to other suggestions.

@nilshempelmann
Copy link
Member

personally I prefer different processes and keep the complexity userfriendly. So two processes for shapefile and WFS input. the " own geoserver " is usable from outside? could be default than.

@nilshempelmann
Copy link
Member

@huard should we merge this? Test are failing caused by eggshell dependencies. The second PR should solve eggshell issues.
feel free to merge if you like

@huard
Copy link
Contributor Author

huard commented Mar 16, 2020

No, still working on it.

@nilshempelmann
Copy link
Member

OK. great. I am going to write tutorial-examples the next days, so take your time. Keep the second PR (eggshell) in mind.

@Zeitsperre
Copy link
Contributor

@huard Anything I can do to help here?

@huard
Copy link
Contributor Author

huard commented Mar 30, 2020

@Zeitsperre I still have messy changes in that branch. Hope to find some time to push a clean version you'd be able to work from.

This current PR uses ocgis, I think you could work on an alternative version within xclim. Start with a lit. review, document options and propose implementation plan in a new issue.

@Zeitsperre
Copy link
Contributor

@huard I'm not sure if I follow. I feel like this already exists in xclim as a set of operations (import DataSet -> subset_shape -> averaging operations using xarray/Pandas-like interface). Something like that should be available in finch at the moment.

@huard
Copy link
Contributor Author

huard commented Mar 30, 2020

@Zeitsperre I'd like the averaging to be part of the WPS, not an operation to be done by the user, otherwise you need to download the data to your computer, average it, then send it back to the Raven server for modeling.

@Zeitsperre
Copy link
Contributor

OK. I imagine that this could be integrated into Finch, based on https://github.com/bird-house/finch/blob/master/finch/processes/wps_xsubset_polygon.py. In that case we just need an averaging process?

@huard
Copy link
Contributor Author

huard commented Mar 30, 2020

Yes, but that accounts for the variable grid size and percent area where the grid overlaps the region.

@huard huard marked this pull request as draft May 13, 2020 12:52
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.

Average over polygon subset
3 participants