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

♻️ Refactor DatashaderRasterizer to be up front about datapipe lengths #39

Merged
merged 1 commit into from
Aug 19, 2022

Conversation

weiji14
Copy link
Owner

@weiji14 weiji14 commented Aug 19, 2022

Check during initialization of DatashaderRasterizerIterDataPipe on whether the input canvas and vector datapipes have compatible lengths. This is better than finding out that the zip function doesn't work when the datapipe is being iterated over. Added a unit test to cover the 3:2 ratio case and documented why the ValueError is raised on unmatched lengths. Also renamed the previous ValueError on unsupported geometry types to NotImplementedError to avoid confusion.

Motivated by issues discovered in #31 (see af489a8) when some DataPipe functions like https://pytorch.org/data/0.4/generated/torchdata.datapipes.iter.BatchMapper.html do not return any lengths.

Patches #35.

Check during initialization of DatashaderRasterizerIterDataPipe on whether the input canvas and vector datapipes have compatible lengths. This is better than finding out that the zip function doesn't work when the datapipe is being iterated over. Added a unit test to cover the 3:2 ratio case and documented why the ValueError is raised on unmatched lengths. Also renamed the previous ValueError on unsupported geometry types to NotImplementedError to avoid confusion.
@weiji14 weiji14 added the bug Something isn't working label Aug 19, 2022
@weiji14 weiji14 added this to the 0.3.0 milestone Aug 19, 2022
@weiji14 weiji14 self-assigned this Aug 19, 2022
@weiji14 weiji14 mentioned this pull request Aug 19, 2022
9 tasks
@weiji14 weiji14 merged commit 23c6ac4 into main Aug 19, 2022
@weiji14 weiji14 deleted the refactor/datashader-rasterizer-lengths branch August 19, 2022 02:31
weiji14 added a commit that referenced this pull request May 14, 2023
Used to wrong boolean operator (should be and, not or) which meant DatashaderRasterizer didn't work when the vector_datapipe was length 1 and canvas_datapipe was length 2 or more. This prevents false errors like `ValueError: Unmatched lengths for the canvas datapipe (XarrayCanvasIterDataPipe) and vector datapipe (PyogrioReaderIterDataPipe). The vector datapipe's length (1) should either be (1) to allow for broadcasting, or match the canvas datapipe's length of (2)`.

Patches the bugfix at #39 from zen3geo v0.3.0. Also updated a unit test to use a 2:1 canvas:vector ratio to prevent regression.
weiji14 added a commit that referenced this pull request May 14, 2023
* 🐛 Fix DatashaderRasterizer to allow N:1 instead of just 1:1

Used to wrong boolean operator (should be and, not or) which meant DatashaderRasterizer didn't work when the vector_datapipe was length 1 and canvas_datapipe was length 2 or more. This prevents false errors like `ValueError: Unmatched lengths for the canvas datapipe (XarrayCanvasIterDataPipe) and vector datapipe (PyogrioReaderIterDataPipe). The vector datapipe's length (1) should either be (1) to allow for broadcasting, or match the canvas datapipe's length of (2)`.

Patches the bugfix at #39 from zen3geo v0.3.0. Also updated a unit test to use a 2:1 canvas:vector ratio to prevent regression.

* 📌 Temporarily pin numpy<1.24 to prevent AttributeError

The latest numpy=1.24 version installed via readthedocs is incompatible with datashader=0.14.4, resulting in an error like `AttributeError: module 'numpy' has no attribute 'warnings'`. This was reported in holoviz/datashader#1209 and fixed in holoviz/datashader#1176, but will need to wait for datashader>=0.14.5 to be released.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant