Fix rasterizing a Path with categorical values including empty strings #6429
Conversation
holoviews/operation/datashader.py
Outdated
| @@ -448,6 +448,12 @@ def _apply_datashader(self, dfdata, cvs_fn, agg_fn, agg_kwargs, x, y): | |||
| val = val.astype(np.float64) | |||
| val[neg1] = np.nan | |||
| agg[col] = ((y.name, x.name), val) | |||
|
|
|||
| if isinstance(agg_fn, ds.count_cat): | |||
There was a problem hiding this comment.
Hopefully a good place to add this.
There was a problem hiding this comment.
Placement looks fine to me. We need to generalize this to ds.by:
| if isinstance(agg_fn, ds.count_cat): | |
| if isinstance(agg_fn, ds.by): |
We could parameterize the test.
| # [0, 1]], | ||
| # ]) | ||
| # expected = ImageStack((x, y, array), vdims=['a', 'b']) | ||
| # self.assertEqual(rasterized, expected) |
There was a problem hiding this comment.
@hoxbro I commented out this code for now as I realized that compare.py has not been updated to compare ImageStack objects part of the test suite. Is this intentional? Not that this part of the test is not strictly required, I worked on this fix together with @Azaya89 and ended up showing him that self.assertEqual is extended by HoloViews. Ultimately, the real check is that there's no error raised and no empty string in the vdims.
There was a problem hiding this comment.
It was not intentional. At some point, I want to transition away from using the stdlib unittest framework.
Can you uncomment this and do some simple assert instead?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6429 +/- ##
==========================================
- Coverage 88.51% 88.51% -0.01%
==========================================
Files 323 323
Lines 68597 68612 +15
==========================================
+ Hits 60722 60733 +11
- Misses 7875 7879 +4 ☔ View full report in Codecov by Sentry. |
hoxbro
force-pushed
the
fix_rasterize_path_sep_empty_string
branch
from
e219e4f
to
01ed576
Compare
Fixes #6326