snap-engine icon indicating copy to clipboard operation
snap-engine copied to clipboard
verified publisher icon senbox-org

Add a flag to allow for Geoid-height on NaN points in DEM generation

Open reinieroost opened this issue 8 years ago • 8 comments

This allows to avoid NaN values at places where the DEM is NaN, by using Geoid heights instead. This is very favorable for coastal regions. It is a switch that does not affect the default.

I will also update the UI (snap-desktop), see pull-request.

reinieroost avatar Sep 22 '17 14:09 reinieroost

Link to desktop PR: https://github.com/senbox-org/snap-desktop/pull/40

marpet avatar Sep 24 '17 07:09 marpet

Thanks for this PR. That's great. User contribution is always welcome. I think we can't consider this for SNAP 6 release any more, but probably for a later update.

Thanks

marpet avatar Sep 24 '17 07:09 marpet

The snap-desktop pull request has already been merged but not this one on snap-engine. Therefore, AddElevationUI currently breaks because the fillWithGeod parameter doesn't exist in AddElevationOp.

Also there are spelling errors for fillWithGeod.

lveci avatar Oct 04 '17 18:10 lveci

@jun--lu Why have you reverted the PR snap-desktop#40 by this https://github.com/senbox-org/snap-desktop/pull/41? Why not simply applying this PR and fix the typo?

marpet avatar Oct 08 '17 13:10 marpet

Hi Marco,

By accident I merged the pull request under snap/desktop on AddElevationUI to master and I did not know that there is anther pull request under snap/engine on AddElevationOp until Luis pointed out. I've tried to merge AddElevationOp to master, then I realised that you want to make this change after V6.0 release (your email on Set. 24 ). Therefore, I reversed the merge.

Jun

jun--lu avatar Oct 09 '17 22:10 jun--lu

Jun,

if this PR is tested and does not break existing processing chains I’m fine with the merge of it. I only said that we probably accept this PR only after 6.0 release because we had several other high priority things to fix first. If you think this is a good one, please go ahead.

marpet avatar Oct 10 '17 07:10 marpet

Hi Marco, As I've said I made the merge really by accident and I don't think I have time to check the code and test it now. It's better to merge it after the release when we have time to deal with things like this. Jun

jun--lu avatar Oct 10 '17 14:10 jun--lu

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Jun 14 '23 08:06 CLAassistant