Allow SPW as provider#73
Conversation
[ENH] Lower case provider to allign with existing
|
If you would have the time to update docs/tests, note the adjustment in #76 to enlist providers. |
|
@TimFranken after the recent updates to also support VMM Grid data and HIC ensembles thanks to @barisoztas , I'm wondering if you would have the time and ability to update the PR and add support for SPW in short notice? This wold enable us to combine it as well in a new release of pywaterinfo. In practice, an update is needed in terms of the provider specification, see https://github.com/fluves/pywaterinfo/blob/master/src/pywaterinfo/waterinfo.py#L39 and in terms of testing addition on supported providers, see https://github.com/fluves/pywaterinfo/blob/master/tests/test_waterinfo.py#L33 and for the functions that are supported by SPW, adding parametrized tests for spw and spw cached connections, e.g. https://github.com/fluves/pywaterinfo/blob/master/tests/test_waterinfo.py#L661 If you are not able, we can check for other resources to give it a push towards the release |
|
@stijnvanhoey , thanks for the further clarifications. I'll take a look at it and try to update the PR this week. Good news is that they now also enable authentication so it should be even more straightforward to add them. |
|
@stijnvanhoey , I've upgraded the implemenation to allign with the recent updates and added tests. I did not yet add anything into the documentation or readme as there are a number of ways this could be added so leave that up to you. Feel free to reach out in case anything else is required (or issues). |
There was a problem hiding this comment.
Dear @TimFranken, thanks for the edits and extending pywaterinfo! All looks good and clear 👍
stijnvanhoey
left a comment
There was a problem hiding this comment.
CI is failing to repeated connectivity issues, so I'll merge anyway. I'll provide a follow up PR myself that add the information on the new provider SPW in the sphinx documentation.
Allow SPW as provider to fetch data from https://hydrometrie.wallonie.be/.
Related to #72
No additional tests are written and documentation is not yet updated to include the new provider.