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

Create a developer guide #496

Merged
merged 21 commits into from
Apr 14, 2023
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
2e6228b
Initial setup
tim-vd-aardweg Mar 30, 2023
cf80532
Added the developer guide to mkdocs
tim-vd-aardweg Mar 30, 2023
ac38749
Refer to the new developer guide in the existing docs.
tim-vd-aardweg Mar 30, 2023
dacce8e
Added a short introduction to the document and added information on h…
tim-vd-aardweg Mar 30, 2023
d136d89
Added initial text for keeping the documentation updated for the API …
tim-vd-aardweg Mar 30, 2023
8fb7819
Merge branch 'main' into docs/481_create_developer_guide
priscavdsluis Mar 30, 2023
a5f8b52
Add some info about updating the Excel sheet with supported DHYDRO fu…
tim-vd-aardweg Mar 30, 2023
017b37c
Merge branch 'docs/481_create_developer_guide' of https://github.com/…
tim-vd-aardweg Mar 30, 2023
be77af3
Remove part about docstrings for now
tim-vd-aardweg Mar 31, 2023
6765bc7
Merge branch 'main' into docs/481_create_developer_guide
priscavdsluis Apr 4, 2023
1946530
Removed developers guide from online documentation
tim-vd-aardweg Apr 7, 2023
3deb863
Removed developers guide from online documentation
tim-vd-aardweg Apr 7, 2023
cd0d2b3
Removed the part about testing models, parsers and serializers.
tim-vd-aardweg Apr 7, 2023
921103e
Merge branch 'main' into docs/481_create_developer_guide
MRVermeulenDeltares Apr 11, 2023
afb69d2
Implemented review comments
tim-vd-aardweg Apr 13, 2023
4e33459
Implemented review comments
tim-vd-aardweg Apr 13, 2023
e92911d
Implemented review comments
tim-vd-aardweg Apr 13, 2023
afcb455
Implemented review comments
tim-vd-aardweg Apr 13, 2023
7dc2cb8
Implemented review comments
tim-vd-aardweg Apr 13, 2023
aee2dd4
Implemented review comments
tim-vd-aardweg Apr 13, 2023
3704f7f
Merge branch 'main' into docs/481_create_developer_guide
MRVermeulenDeltares Apr 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/guides/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ The merge commit message should adhere to the [conventional commit guidelines](h
### Coding guidelines
* If there is code that needs to be tested, there should be tests written for it.
* If there are any additions or changes to the public API, the documentation should be updated.
* Files should be added to the appropriate folder to keep modules and objects within the correct scope.
* Files should be added to the appropriate folder to keep modules and objects within the correct scope.
MRVermeulenDeltares marked this conversation as resolved.
Show resolved Hide resolved

## Releasing
### Making a release on GitHub and PyPi
Expand Down
49 changes: 49 additions & 0 deletions docs/guides/developers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Developer guide
MRVermeulenDeltares marked this conversation as resolved.
Show resolved Hide resolved
This guide contains some more in-depth guidelines, tasks and examples that developers should adhere to when working on hydrolib-core.
In addition to being a useful resource for developing, this documentation can also be helpful for code reviews.

## Explicitly exposing the public API
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
Whenever you add new functionality that should (or could) be used by the users, make sure you explicitly expose them in the correct `__init__.py` files. To read from and write to
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved

For example, you have developed a new `TimModel` class that represents a `.tim` file. To read from and write to the underlying `.tim` files, you have also created a new `TimParser` class and `TimSerializer` class. While the `TimModel` class should be publicly exposed to the users, because it is part of the public API, the parser and serializer classes are implementation details that users should not be concerned with. They should therefore not be publicly exposed to the users.

To expose the `TimModel` explicitly to our users, you have to update both the `hydrolib/core/dflowfm/tim/__init__.py` (assuming you created a new `tim` folder for this new functionality) and the `hydrolib/core/dflowfm/__init__.py`:

```python
# hydrolib/core/dflowfm/tim/__init__.py
from .models import TimModel

__all__ = [
"TimModel",
]
```

```python
# hydrolib/core/dflowfm/__init__.py
...
...
from .tim import *
```

## Updating the documentation
In the hydrolib-core repository there are several reference files that are used to automatically generate the [API reference](../reference/api.md). If you add or make changes to the existing API, always make sure the API reference is still up to date.

For example, let's use the example that was used above, where the `TimModel` was introduced in hydrolib-core. Since this is new functionality that affects the API, we have to update the reference files. The API reference files are located at `docs\reference`. Since the `TimModel` was newly introduced, we should create a new file named `tim.md`. The contents of this file include a short description about what the '.tim' file is and what it is used for, followed by the actual API. You do not have to manually write the API, that is done for us by mkdocs. Since the newly introduced `TimParser` and `TimSerializer` are not part of the public API, they should not be added to the reference file.
MRVermeulenDeltares marked this conversation as resolved.
Show resolved Hide resolved

An example of such an `.md` can be found below:

```md
MRVermeulenDeltares marked this conversation as resolved.
Show resolved Hide resolved
# Timeseries .tim files
Timeseries .tim files are timeseries input files
for a [D-Flow FM](glossary.md#d-flow-fm) model.
The support of .tim files is discontinued and are replaced by the [.bc file](#bc-file).
MRVermeulenDeltares marked this conversation as resolved.
Show resolved Hide resolved

They are represented by the class below.

## Model
::: hydrolib.core.dflowfm.tim.models

```

### Updating the functionalities sheet
To generate a list of D-HYDRO functionalities, grouped by kernel, and the current status of support inside hydrolib-core, we use an Excel sheet. This Excel sheet can be found at `docs\topics\dhydro_support_hydrolib-core.xlsx`. Each time you add support for new D-HYDRO functionalities, this Excel file has to be updated. Detailed information on how to update the Excel file correctly can be found inside the Excel file itself.
2 changes: 1 addition & 1 deletion mkdocs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ nav:
- "guides/documentation.md"
- Support:
- guides/gettinghelp.md
- Tutorials:
- Tutorials:
- tutorials/tutorials.md
- tutorials/build_basic_model.ipynb
- tutorials/loading_and_saving_a_model.md
Expand Down