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

Data source type requirement in setup_ methods is not clear #225

Closed
1 task done
hboisgon opened this issue Nov 28, 2023 · 2 comments · Fixed by #227
Closed
1 task done

Data source type requirement in setup_ methods is not clear #225

hboisgon opened this issue Nov 28, 2023 · 2 comments · Fixed by #227
Assignees
Labels
documentation Improvements or additions to documentation needs refinement issue still needs refinement

Comments

@hboisgon
Copy link
Contributor

HydroMT-Wflow version checks

  • I have checked that the issue still exists on the latest versions of the docs on main here

Kind of issue

Docs are unclear

Location of the documentation

Each of the individual setup_ methods in the wflow.py script (API documentation)

Documentation problem

In the setup_ methods it is often unclear if the expected input data type is rasterdataset, geodataset, dataframe or geodataframe for example. I think this should be made clear in the documentation so that users know which type of data to prepare for model building.

Suggested fix for documentation

Example for setup_rivers:

From

        hydrography_fn : str, Path
            Name of data source for hydrography data.
            Must be same as setup_basemaps for consistent results.


            * Required variables: [flwdir, uparea, elevtn]
            * Optional variables: [rivwth, qbankfull]
        river_geom_fn : str, Path, optional
            Name of data source for river data.


            * Required variables: [rivwth, qbankfull]

To

        hydrography_fn : str, Path
            Name of RasterDataset source for hydrography data.
            Must be same as setup_basemaps for consistent results.


            * Required variables: [flwdir, uparea, elevtn]
            * Optional variables: [rivwth, qbankfull]
        river_geom_fn : str, Path, optional
            Name of GeoDataFrame source for river data.


            * Required variables: [rivwth, qbankfull]

Note: maybe a good moment to also update the docs that hydrography_fn can be data source in data catalog, xarray.Dataset or direct path to data?

@hboisgon hboisgon added documentation Improvements or additions to documentation needs refinement issue still needs refinement labels Nov 28, 2023
@hboisgon
Copy link
Contributor Author

Kind of related: some variables or dimensions names are missing from the docs. Known ones:

  • cyclic time dimension (values 1 to 12) for LAI in setup_laimaps
  • cyclic time dimension (values 1 to 12) for precip_clim_fn in setup_precip_forcing
  • elevtn [m] required varibales for dem_forcing_fn in setup_temp_pet_forcing
  • qbankfull unit [m3/s] is not documented in hydromt core (yet) for setup_rivers
  • basin_index_fn ? + it is optional and not required anymore.

@hboisgon
Copy link
Contributor Author

This issue is partly related to #157

@hboisgon hboisgon self-assigned this Dec 13, 2023
hboisgon added a commit that referenced this issue Dec 14, 2023
@hboisgon hboisgon mentioned this issue Dec 14, 2023
4 tasks
@JoostBuitink JoostBuitink added this to the 2024 - high priority milestone Jan 24, 2024
dalmijn added a commit that referenced this issue Feb 13, 2024
* Update setup_gauges docs #198

* fix pet_method error #201

* improve setup methods docstrings #225

* update changelog

* Indentation error

* Remove optional for 'temp_pet_fn' in docstring

---------

Co-authored-by: Brendan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation needs refinement issue still needs refinement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants