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

Fix NaNs in "longyearbyen_ddem" example #224

Merged
merged 13 commits into from
Nov 8, 2021

Conversation

rhugonnet
Copy link
Contributor

@rhugonnet rhugonnet commented Oct 14, 2021

@rhugonnet rhugonnet changed the title Fix NaNs in 'longyear_ddem' example Fix NaNs in "longyearbyen_ddem" example Oct 14, 2021
@rhugonnet
Copy link
Contributor Author

For opening issue: we don't want to repeat this operation after every Coreg operation to preserve nodata along calculations:

aligned_raster.data[~np.isfinite(aligned_raster)] = reference_raster.nodata
diff = reference_raster - \
           gu.Raster.from_array(aligned_raster.data, transform=reference_raster.transform, crs=reference_raster.crs, nodata=reference_raster.nodata)

@rhugonnet
Copy link
Contributor Author

rhugonnet commented Nov 7, 2021

It seems that GlacioHack/geoutils#257 fixed the issues that appeared in the tests.
We can merge!

@erikmannerfelt
Copy link
Contributor

I'm afraid I don't really understand the fix here! You simplify the syntax and clean up the variogram test, which is great. How does this relate to the nans in the test?

@rhugonnet
Copy link
Contributor Author

@erikmannerfelt The example bug fix is in the example. The rest is a side fix for using the Raster class directly as input, I added a line in the main description of the PR:

  • Also fixes the use of Raster as an input of sample_empirical_variogram with a tiny bug on the ground sampling distance definition fixed, and tests updated

Copy link
Contributor

@erikmannerfelt erikmannerfelt left a comment

Choose a reason for hiding this comment

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

Ah! Weirdly the example code diff didn't load on my GitHub. Now it does, and it looks good!

@rhugonnet rhugonnet merged commit 4542b25 into GlacioHack:main Nov 8, 2021
@rhugonnet rhugonnet deleted the fix_nanexample branch November 8, 2021 10:47
@@ -94,9 +96,10 @@ def process_coregistered_examples(overwrite: bool =False):
nuth_kaab.fit(reference_raster.data, to_be_aligned_raster.data,
inlier_mask=inlier_mask, transform=reference_raster.transform)
aligned_raster = nuth_kaab.apply(to_be_aligned_raster.data, transform=reference_raster.transform)
Copy link
Member

Choose a reason for hiding this comment

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

This line and the few lines below could be simplified now that Coreg methods take Rasters as arguments and with the arithmetic overloading functions !

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.

Example "longyearbyen_ddem" contains NaNs
3 participants