pywr / pycatchmod

A Cython implementation of the rainfall runoff model CATCHMOD (Wilby, 1994)
GNU General Public License v3.0
6 stars 5 forks source link

ability to output area flow timeseries #42

Closed laurenpetch closed 5 years ago

laurenpetch commented 5 years ago

in response to #39

snorfalorpagus commented 5 years ago

This looks good to me (with minor corrections). It would benefit from a test that demonstrates the new functionality. I looked for one to modify, but it looks like we don't currently test the multi-scenario functionality @jetuk?

jetuk commented 5 years ago

I think the multi-scenario thing is tested here:

https://github.com/pywr/pycatchmod/blob/6efd286291aea52b7e5da75aa1b97f39efac27b1/tests/test_catchmod.py#L236

Would be good to parametrize this test to use the new flag with True/False and ensure the output is as expected.

jetuk commented 5 years ago

@laurenpetch is this ready now?

laurenpetch commented 5 years ago

@jetuk yes i think all good now

jetuk commented 5 years ago

I reread. One final comment to use assert_allclose instead of assert_equal in the tests. I'll merge and do a new release when that's done.

laurenpetch commented 5 years ago

Forgot about this in the run up to christmas. Should be all good to go now