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

IJsselmeer model #26

Merged
merged 33 commits into from
Nov 7, 2023
Merged

IJsselmeer model #26

merged 33 commits into from
Nov 7, 2023

Conversation

d2hydro
Copy link
Contributor

@d2hydro d2hydro commented Oct 27, 2023

Notebooks and updates for generating the ijsselmeermodel:

  • included layer in generate_model_id to ensure uniqueness
  • output type annotaties in code_utils as discussed: Code utils #25 (comment)

linked issues:
#3: organize data
#28: build model

Discussion: https://github.com/Deltares/Ribasim-NL/discussions/27

@d2hydro d2hydro mentioned this pull request Oct 27, 2023
@d2hydro
Copy link
Contributor Author

d2hydro commented Oct 27, 2023

@visr
Copy link
Member

visr commented Oct 27, 2023

src/hydamo/hydamo/code_utils.py:48: error: Missing return statement  [return]
src/hydamo/hydamo/code_utils.py:57: error: Missing return statement  [return]

I see both functions have a return statement only under an if block. I think all possible paths need to be covered by a return statement.

src/hydamo/hydamo/code_utils.py:[6](https://github.com/Deltares/Ribasim-NL/actions/runs/6665005976/job/18113733533?pr=26#step:5:7)1: error: Missing type parameters for generic type "dict"  [type-arg]

I think it just prefers Dict where you can specify the key and value types, so not -> dict but, Dict[str, int] or something like that, with from typing import Dict.

@visr visr changed the title Ijsselmeer model IJsselmeer model Oct 30, 2023
environment.yml Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
@visr visr marked this pull request as ready for review November 7, 2023 12:56
@visr visr merged commit ff0e5fe into main Nov 7, 2023
3 checks passed
@visr visr deleted the ijsselmeer_model branch November 7, 2023 12:56
@visr visr mentioned this pull request Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants