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

Improved flw.d8_from_dem method #305

Merged
merged 9 commits into from
Apr 12, 2024
Merged

Improved flw.d8_from_dem method #305

merged 9 commits into from
Apr 12, 2024

Conversation

DirkEilander
Copy link
Contributor

No description provided.

@savente93
Copy link
Contributor

@DirkEilander are you still interested in this functionality? If so I'd urge you to try to get this merged because the further we get with v1 the harder it will be to integrate down the line

@DirkEilander
Copy link
Contributor Author

@DirkEilander are you still interested in this functionality? If so I'd urge you to try to get this merged because the further we get with v1 the harder it will be to integrate down the line

Thanks for the reminder. Yes, this functionality should land in HydroMT. I'll try to squeeze it in some time soon

@savente93 savente93 removed the Stale PR label Apr 8, 2024
@DirkEilander DirkEilander marked this pull request as ready for review April 11, 2024 14:40
@DirkEilander DirkEilander changed the title ENH: more options for flw.flwdir_from_dem Improved flw.d8_from_dem method Apr 11, 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.

Nice that we can use something else than upstream area to burn rivers in the DEM!
PR looks good, just one small check if outlets = idxs_pit that this indeed works with pyflwdir but else can be merged :)

hydromt/flw.py Outdated Show resolved Hide resolved
Co-authored-by: hboisgon <45457510+hboisgon@users.noreply.github.com>
@DirkEilander
Copy link
Contributor Author

Nice that we can use something else than upstream area to burn rivers in the DEM! PR looks good, just one small check if outlets = idxs_pit that this indeed works with pyflwdir but else can be merged :)

I wanted to make it more explicit here what option is selected. I should do the same in pyflwdir. But it already works because in pyflwdir I only check for the outlets="min" case.

@DirkEilander DirkEilander merged commit d8120b7 into main Apr 12, 2024
9 of 10 checks passed
@DirkEilander DirkEilander deleted the local_dem branch April 12, 2024 07:14
@savente93 savente93 mentioned this pull request Jun 14, 2024
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.

None yet

3 participants