JOSS Review - Githubissues

kbonney commented 3 weeks ago

Hi @andreArtelt, nice work on the package and paper. Below are some comments for improving both. The only points that I need resolved to complete my review are marked by a :exclamation:, the rest are suggestions you can take or leave now or later.

Functionality

The software relies on software that isn't actively maintained umsgpack. I don't think this is something that needs to be directly addressed, but I do want to point out that relying on unmaintained packages can cause issues down the line as the package could possibly break on new releases of Python.

Documentation

:exclamation: I haven't found the statement of need anywhere in the docs. I've seen people put this in the repo's README or the landing page of their documentation website.
:exclamation: When I run the test suite all tests pass except for the benchmark tests which seem to get caught up on some request issues. I'll work on this a little bit more but if I can't resolve it I'll open a separate issue about the problem.
My understanding is that the primary use case of EPyT-Flow is to integrate with machine learning, so I think it would be beneficial to include an example of what this might look like. Similarly, those unfamiliar with a REST API would benefit from an example of what they could do with the one you've provided. This could be a concrete example or just an explanation of REST with some suggested use cases.
[x] :exclamation: Contributions guidelines are very minimal. I suggest adding templates for Issues and PRs (you can find plenty of examples online) and also adding some text somewhere in your README or docs addressing how to seek support.

Software Paper

:exclamation: In the section "State of the Field" there is a statement that WNTR does not support quality dynamics, however basic quality functionality is currently available (see https://usepa.github.io/WNTR/waterquality.html) and advanced quality analysis via the MSX module is nearing completion (see https://github.com/USEPA/WNTR/pull/426). Similarly, there are controls available in WNTR (see https://usepa.github.io/WNTR/controls.html). I recommend dropping the mention of quality and controls from the limitations of WNTR. From what I can see, the simulation capabilities of WNTR and EPyT-Flow are pretty similar, so I would focus on the functionality of EPyT-Flow that facilitates systematic generation and simulation of scenarios such as the integration of uncertainty and the Gym environment to differentiate the package from WNTR.
The software mentions seven benchmarks which can be used for evaluating algorithms (line 71-73), but it isn't entirely clear what this means. It would be beneficial to give readers some sort of vision for how you imagine people using these benchmarks. The discussion of uncertainty could also be expanded. I have an AI background so it is clear to me how the uncertainty module would be useful for building datasets, for example, but that might not be obvious to someone without that background.