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

Bugfix setup channels and 2d1d links #133

Merged
merged 25 commits into from
Sep 26, 2024
Merged

Bugfix setup channels and 2d1d links #133

merged 25 commits into from
Sep 26, 2024

Conversation

shartgring
Copy link
Collaborator

@shartgring shartgring commented Aug 6, 2024

Issue addressed

Fixes #131 and #132

Explanation

Some typos and incorrect variable types resulted in errors being raised. This PR solves this by removing the typos and adapting some of the code copied from hydrolib to be able to work with correct variable types. This is not an improvement of the functionality, but merely bigfixing

Checklist

  • Updated tests or added new tests
  • Branch is up to date with main
  • Tests & pre-commit hooks pass
  • Updated documentation if needed <-- default for spacing has been set to np.inf, include this in documentation?
  • Updated changelog.rst if needed <-- should I add specifics on the bugfixes, or can we call it 'bufixing' in general? Or maybe refer to the methods where the bugs have been fixed?

Copy link

sonarcloud bot commented Aug 6, 2024

Copy link
Contributor

@hboisgon hboisgon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! For the changelog you can mention the bugfix for direction 2d1d (#132 ). Was the spacing value in any of the hydromt build/update configuration examples? If so it's good to mention. Can you check that this works also with the PR #139 ? You may have conflicts when merging on unifricttype

@veenstrajelmer veenstrajelmer linked an issue Sep 25, 2024 that may be closed by this pull request
2 tasks
Copy link

sonarcloud bot commented Sep 26, 2024

@shartgring
Copy link
Collaborator Author

@hboisgon I addressed your points! It will not affect the examples as setup_channels is not included in the examples (thus they are not affected by the changes). I will make an issue that it would be good to include it, as such an example can also be used for tests

@shartgring shartgring linked an issue Sep 26, 2024 that may be closed by this pull request
@shartgring shartgring merged commit b1c9caa into main Sep 26, 2024
8 checks passed
@shartgring shartgring deleted the bugfix_setup_channels branch September 26, 2024 12:09
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.

bugs in setup_link1d2d bugs in setup_channels
3 participants