Deltares / hydromt

HydroMT: Automated and reproducible model building and analysis
https://deltares.github.io/hydromt/
MIT License
75 stars 30 forks source link

JOSS Review #260

Closed mcflugen closed 1 year ago

mcflugen commented 1 year ago

@DirkEilander, @hboisgon, @markhegnauer Thank you for your JOSS submission for HydroMT. I think it's a great contribution and I don't see any major changes you need to make. I'll use this issue as a place to provide comments.

In the interest of getting things going, I have some minor edit suggestions to the paper that I'll provided now. The line numbers refer to those in the build of the paper here.

Minor edits:

openjournals/joss-reviews#4897

mcflugen commented 1 year ago

A few additional comments:

DirkEilander commented 1 year ago

Dear @mcflugen,

Many thanks for the useful review which has certainly improved the manuscript! Hereby a reply to your comments:

mcflugen commented 1 year ago

@DirkEilander Sounds good. Thank you!

I found a couple more typos:

Regarding BMI, I was thinking of datasets (i.e. ERA5, WAVEWATCHIII, etc.)—not software models—that expose a BMI through, for example, a Python class. Would it be possible to create a generalized HydroMT data adapter that can gather input data (typically, maybe exclusively, gridded datasets) for a software model from a BMI dataset? And, maybe more importantly, would it be useful? My thinking is that, with such an adapter, it may make it easier for you to add new datasets to your catalog if a BMI has already been implemented for those datasets. I'm really just thinking out loud here, no need to address this in your paper.

DirkEilander commented 1 year ago

Thanks @mcflugen for typos and suggestion!

I'm not too familiar with the BMI datasets, but if I understand it correctly it's basically a common API to retrieve datasets from different APIs/sources, am I correct? I do think that would be compatible with the HydroMT RasterDataset adapter and it becomes especially useful if different datasets can be accessed in the same way. Where can I find a full overview of the different datasets which are available? This overview on the PyMT documentation seems to be outdated? I've opened a new issue to discuss this further: https://github.com/Deltares/hydromt/issues/271.