-
-
Notifications
You must be signed in to change notification settings - Fork 366
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
Canvas.raster fixes #862
Canvas.raster fixes #862
Conversation
It seems that adding |
to work around conda solver error
Adding Now, here are the
|
Looks like those tests have been failing for a while:
Different tests were failing in 0.6.6 and in previous versions, so it's hard to find some pristine reference to compare against. |
@philippjfr, I used PR #762 has a note from you saying "Upsampling with 'linear' interpolation does not yet use correct padding so boundaries are wrong", which sounds suspiciously like what this is testing, though this test appears to be downsampling a 4x3 array down to 3x3, not upsampling. The PR also has a comment from me saying "it would be difficult for me to spot an off-by-one error...". :-). I pulled out the test code as:
What this test is verifying is that when calling
the result is a 3x3 array padded with the indicated value:
After #762, the calculated coordinates, dims, and resolution are the same as before, but the actual data array is now too small:
Any idea how to fix the padding? |
Afaik that note should only apply to dask resampling where offsets are actually used but I could have accidentally introduced some regression. Will have a look. |
Thanks. It looks to me like the first two failing tests have the same underlying cause (as their code differs only by data type). The third one (
but getting
|
Two actual bugs here:
|
1ece28f
to
7ccc3d6
Compare
Thanks @philippjfr and @jonmmease ! To summarize the changes made in this PR:
It may be possible to optimize the raster mode calculations by allocating one values/frequencies buffer per thread and then emptying it out between output pixels, but this would make the code more complex and harder to reason about, and may not provide any performance benefit. Still, if someone want to optimize mode calculations, they could consider implementing and benchmarking that. Meanwhile, this approach should be safe and is probably reasonably fast. |
It looks like the tests in
datashader/tests/test_raster.py
have been getting skipped on CI becauserasterio
was not being installed in the test environment.