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

Support summary containing by reduction with other reductions #1257

Merged
merged 1 commit into from
Jul 25, 2023
Merged

Support summary containing by reduction with other reductions #1257

merged 1 commit into from
Jul 25, 2023

Conversation

ianthomas23
Copy link
Member

Fixes #1256.

Adds support for the following combinations of summary and by reductions:

  1. summary(by("cat1"), other_reduction) and with the order swapped.
  2. summary(by("cat1"), by("cat1"))
  3. summary(by("cat1"), by("cat2"))

The biggest change here is in the handling of categorical columns in compiler.py make_append(). Previously the categorical flag was constant for all reductions within a summary, now it is checked for each constituent reduction. Where categorical columns are reused, they are only extracted once.

Also, the addition of kwargs to xr.DataArray in by.finalize has been modified to follow the same approach as in where reductions, which is to add the new categorical coords/dims to a copy of the kwargs, not modify them in-place. Hence each constituent reduction of a summary can have different categorical coordinates from others.

Tested on CPU and GPU, with and without Dask.

@ianthomas23 ianthomas23 modified the milestones: v0.15.x, v0.15.2 Jul 24, 2023
@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Merging #1257 (e45fee4) into main (4a1c3fc) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1257      +/-   ##
==========================================
- Coverage   83.44%   83.41%   -0.04%     
==========================================
  Files          35       35              
  Lines        8882     8886       +4     
==========================================
  Hits         7412     7412              
- Misses       1470     1474       +4     
Impacted Files Coverage Δ
datashader/compiler.py 88.55% <100.00%> (+0.17%) ⬆️
datashader/reductions.py 77.80% <100.00%> (-0.27%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ianthomas23 ianthomas23 merged commit 7aaba9f into holoviz:main Jul 25, 2023
@ianthomas23 ianthomas23 deleted the 1256_summary_multiple_by branch July 25, 2023 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

summary reduction containing a by reduction and any other reduction fails
1 participant