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

Calculate raster coords identically to other primitives #1046

Merged
merged 2 commits into from
Feb 8, 2022
Merged

Calculate raster coords identically to other primitives #1046

merged 2 commits into from
Feb 8, 2022

Conversation

ianthomas23
Copy link
Member

Fixes issue #1038, supersedes PR #1042.

Calculation of x, y coords for rasters was using a different code path to that used by other geometric primitives, leading to very small differences (~1e-15) that can cause problems when using tf.stack.

This PR removes the utils.compute_coords function that was only used by Canvas.raster and instead uses the same combination of Axis.compute_scale_and_translation and Axis.compute_index as the other primitives, so that now the coordinates are identical.

I have slightly refactored where this code is called in Canvas.raster as the x and y coords are now independent, and replaced use of np.isclose(...).all() with np.allclose(...) which is simpler.

@philippjfr
Copy link
Member

Much better, thank you!

@jbednar
Copy link
Member

jbednar commented Feb 8, 2022

Fabulous! This is what I was looking for all along. :-)

I keep seeing test jobs being cancelled, but I haven't been cancelling them, and am not sure what is doing that.

@maximlt
Copy link
Member

maximlt commented Feb 8, 2022

Seems like the timeout of 40 minutes is not enough on MacOs. I'll increase it in #1045.

@jbednar jbednar merged commit 96d876a into holoviz:master Feb 8, 2022
@ianthomas23 ianthomas23 deleted the raster_coords branch July 19, 2023 09:54
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

4 participants