Add highlighting feature for data elements - #30
Merged
edmundmiller merged 6 commits intoNov 17, 2025
Conversation
Add a new `highlight` parameter to `UpSetAltair()` that allows users to programmatically highlight specific intersections without requiring interactive hover. Features: - highlight="least": highlights the smallest intersection - highlight="greatest": highlights the largest intersection - highlight=<index>: highlights a specific intersection by index (0-based) - highlight=[indices]: highlights multiple intersections - highlight=None: default hover behavior (unchanged) This addresses the need to specify which sets to highlight in static visualizations or when creating non-interactive charts. Added comprehensive tests and documentation examples demonstrating all highlighting modes.
Replace superficial "does it exist" tests with proper validation tests that verify the CORRECT intersections are highlighted. Changes: - test_highlight_least: now verifies the highlighted intersection has the actual minimum count - test_highlight_greatest: now verifies the highlighted intersection has the actual maximum count - test_highlight_specific_index: now verifies the correct intersection_id is selected for the given index - test_highlight_multiple_indices: now verifies all requested indices map to the correct intersection_ids These tests would now catch bugs like: - Swapped least/greatest logic - Off-by-one errors in indexing - Wrong intersection being highlighted - Using degree instead of count for min/max
Improvements: - Extract _determine_highlighted_intersections() helper function for better testability and code organization - Use Literal types for type safety (only "least" and "greatest" allowed) - Reduce main function complexity from ~330 lines to ~300 lines - Make highlight determination logic more reusable and maintainable No functional changes - all tests pass.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
Pull Request Overview
This pull request adds a programmatic highlighting feature to UpSet plots, allowing users to automatically highlight specific intersections based on size ("least"/"greatest"), single index, or multiple indices, as an alternative to the default interactive hover behavior.
Key changes:
- Added
highlightparameter toUpSetAltairfunction with support for multiple highlight modes - Implemented
_determine_highlighted_intersectionshelper function to compute which intersections to highlight - Added comprehensive test coverage for all highlight modes and validation
Reviewed Changes
Copilot reviewed 8 out of 13 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
altair_upset/upset.py |
Added highlight parameter and _determine_highlighted_intersections function for programmatic intersection highlighting |
tests/test_advanced_features.py |
Added 11 test cases covering all highlight modes and error conditions |
docs/examples/advanced_features.rst |
Added documentation with examples for each highlight mode |
| Snapshot JSON files | Updated parameter/view names due to internal changes (param_23→param_33, view_45→view_65, etc.) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+46
to
+53
| if highlight < len(intersections): | ||
| return [intersections.iloc[highlight]["intersection_id"]] | ||
| return [] | ||
| else: # isinstance(highlight, list) | ||
| return [ | ||
| intersections.iloc[i]["intersection_id"] | ||
| for i in highlight | ||
| if i < len(intersections) |
There was a problem hiding this comment.
When an integer index is out of bounds, the function silently returns an empty list. This could lead to confusing behavior where no intersection is highlighted without any indication of why. Consider raising an IndexError or ValueError with a descriptive message instead.
Suggested change
| if highlight < len(intersections): | |
| return [intersections.iloc[highlight]["intersection_id"]] | |
| return [] | |
| else: # isinstance(highlight, list) | |
| return [ | |
| intersections.iloc[i]["intersection_id"] | |
| for i in highlight | |
| if i < len(intersections) | |
| if 0 <= highlight < len(intersections): | |
| return [intersections.iloc[highlight]["intersection_id"]] | |
| raise IndexError( | |
| f"Highlight index {highlight} is out of bounds for intersections of length {len(intersections)}." | |
| ) | |
| else: # isinstance(highlight, list) | |
| invalid_indices = [i for i in highlight if not (0 <= i < len(intersections))] | |
| if invalid_indices: | |
| raise IndexError( | |
| f"Highlight indices {invalid_indices} are out of bounds for intersections of length {len(intersections)}." | |
| ) | |
| return [ | |
| intersections.iloc[i]["intersection_id"] | |
| for i in highlight |
Comment on lines
+50
to
+53
| return [ | ||
| intersections.iloc[i]["intersection_id"] | ||
| for i in highlight | ||
| if i < len(intersections) |
There was a problem hiding this comment.
When a list contains out-of-bounds indices, they are silently filtered out. This could cause confusion if a user expects all specified intersections to be highlighted. Consider raising an error if any index is out of bounds, or at least logging a warning.
Suggested change
| return [ | |
| intersections.iloc[i]["intersection_id"] | |
| for i in highlight | |
| if i < len(intersections) | |
| out_of_bounds = [i for i in highlight if i < 0 or i >= len(intersections)] | |
| if out_of_bounds: | |
| raise ValueError( | |
| f"The following indices are out of bounds for intersections (valid range: 0 to {len(intersections)-1}): {out_of_bounds}" | |
| ) | |
| return [ | |
| intersections.iloc[i]["intersection_id"] | |
| for i in highlight |
Comment on lines
+236
to
+239
| (p for p in params | ||
| if "select" in p and "fields" in p["select"] | ||
| and "intersection_id" in p["select"]["fields"] | ||
| and "value" in p), |
There was a problem hiding this comment.
[nitpick] The generator expression spans multiple lines without proper indentation continuation. Consider formatting it consistently across all test functions or extracting it into a helper function to reduce code duplication (appears in lines 236-240, 282-286, 327-331, 367-371).
Address Copilot feedback from PR #30 to improve error handling: 1. Raise IndexError for out-of-bounds single index (was silent failure) - Before: highlight=999 → silently returns empty list - After: highlight=999 → raises IndexError with clear message 2. Raise ValueError if any list indices are out of bounds (was silent filtering) - Before: highlight=[0, 1, 999] → silently ignores 999 - After: highlight=[0, 1, 999] → raises ValueError listing invalid indices 3. Extract test helper function to reduce code duplication - Added _get_color_selection_param() to eliminate repeated code - Improves test maintainability and readability These changes make the API more Pythonic by failing explicitly rather than silently, helping users catch mistakes early.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
edmundmiller
had a problem deploying
to
testpypi
November 12, 2025 02:51 — with 
GitHub Actions
Failure
GitHub Actions
Failure
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 8 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
edmundmiller
had a problem deploying
to
testpypi
November 16, 2025 18:34 — with
GitHub Actions
Failure
GitHub Actions
Failure
edmundmiller
force-pushed
the
claude/add-highlight-dots-bars-011CV2voFCuSPAJYUHjLRjpD
branch
from
November 16, 2025 18:36
03e94da to
00e7378
Compare
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
edmundmiller
had a problem deploying
to
testpypi
November 16, 2025 18:37 — with
GitHub Actions
Failure
GitHub Actions
Failure
|
Claude Code is working… I'll analyze this and get back to you. |
edmundmiller
had a problem deploying
to
testpypi
November 16, 2025 18:43 — with
GitHub Actions
Failure
GitHub Actions
Failure
edmundmiller
pushed a commit
that referenced
this pull request
Nov 17, 2025
Address Copilot feedback from PR #30 to improve error handling: 1. Raise IndexError for out-of-bounds single index (was silent failure) - Before: highlight=999 → silently returns empty list - After: highlight=999 → raises IndexError with clear message 2. Raise ValueError if any list indices are out of bounds (was silent filtering) - Before: highlight=[0, 1, 999] → silently ignores 999 - After: highlight=[0, 1, 999] → raises ValueError listing invalid indices 3. Extract test helper function to reduce code duplication - Added _get_color_selection_param() to eliminate repeated code - Improves test maintainability and readability These changes make the API more Pythonic by failing explicitly rather than silently, helping users catch mistakes early.
edmundmiller
deleted the
claude/add-highlight-dots-bars-011CV2voFCuSPAJYUHjLRjpD
branch
November 17, 2025 22:13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.