Skip to content

Commit 50bc91d

Browse files
claudeedmundmiller
claude
authored and
edmundmiller
committed
fix: raise errors for out-of-bounds highlight indices
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.
1 parent efb8c6b commit 50bc91d

7 files changed

Lines changed: 197 additions & 191 deletions

‎altair_upset/upset.py‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,13 @@ def _determine_highlighted_intersections(
2626
-------
2727
list of float
2828
List of intersection_ids to highlight
29+
30+
Raises
31+
------
32+
IndexError
33+
If a single integer index is out of bounds
34+
ValueError
35+
If any index in a list is out of bounds
2936
"""
3037
# Get unique intersections with their counts
3138
intersections = (
@@ -43,15 +50,21 @@ def _determine_highlighted_intersections(
4350
max_idx = intersections["count"].idxmax()
4451
return [intersections.loc[max_idx, "intersection_id"]]
4552
elif isinstance(highlight, int):
46-
if highlight < len(intersections):
47-
return [intersections.iloc[highlight]["intersection_id"]]
48-
return []
53+
if highlight >= len(intersections):
54+
raise IndexError(
55+
f"highlight index {highlight} is out of bounds for "
56+
f"{len(intersections)} intersections"
57+
)
58+
return [intersections.iloc[highlight]["intersection_id"]]
4959
else: # isinstance(highlight, list)
50-
return [
51-
intersections.iloc[i]["intersection_id"]
52-
for i in highlight
53-
if i < len(intersections)
54-
]
60+
# Validate all indices first
61+
invalid_indices = [i for i in highlight if i >= len(intersections)]
62+
if invalid_indices:
63+
raise ValueError(
64+
f"highlight indices {invalid_indices} are out of bounds for "
65+
f"{len(intersections)} intersections"
66+
)
67+
return [intersections.iloc[i]["intersection_id"] for i in highlight]
5568

5669

5770
class UpSetChart:

‎tests/__snapshots__/test_covid_mutations/test_covid_mutations[vega_spec].json‎

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
"params": [
3737
{
3838
"bind": "legend",
39-
"name": "param_33",
39+
"name": "param_35",
4040
"select": {
4141
"fields": [
4242
"set"
@@ -51,7 +51,7 @@
5151
]
5252
},
5353
{
54-
"name": "param_34",
54+
"name": "param_36",
5555
"select": {
5656
"fields": [
5757
"intersection_id"
@@ -195,12 +195,12 @@
195195
},
196196
{
197197
"filter": {
198-
"param": "param_33"
198+
"param": "param_35"
199199
}
200200
},
201201
{
202202
"filter": {
203-
"param": "param_33"
203+
"param": "param_35"
204204
}
205205
},
206206
{
@@ -272,7 +272,7 @@
272272
"condition": {
273273
"test": {
274274
"not": {
275-
"param": "param_34"
275+
"param": "param_36"
276276
}
277277
},
278278
"value": "#3A3A3A"
@@ -376,12 +376,12 @@
376376
},
377377
{
378378
"filter": {
379-
"param": "param_33"
379+
"param": "param_35"
380380
}
381381
},
382382
{
383383
"filter": {
384-
"param": "param_33"
384+
"param": "param_35"
385385
}
386386
},
387387
{
@@ -450,7 +450,7 @@
450450
"condition": {
451451
"test": {
452452
"not": {
453-
"param": "param_34"
453+
"param": "param_36"
454454
}
455455
},
456456
"value": "#3A3A3A"
@@ -553,12 +553,12 @@
553553
},
554554
{
555555
"filter": {
556-
"param": "param_33"
556+
"param": "param_35"
557557
}
558558
},
559559
{
560560
"filter": {
561-
"param": "param_33"
561+
"param": "param_35"
562562
}
563563
},
564564
{
@@ -627,7 +627,7 @@
627627
"condition": {
628628
"test": {
629629
"not": {
630-
"param": "param_34"
630+
"param": "param_36"
631631
}
632632
},
633633
"value": "#3A3A3A"
@@ -728,12 +728,12 @@
728728
},
729729
{
730730
"filter": {
731-
"param": "param_33"
731+
"param": "param_35"
732732
}
733733
},
734734
{
735735
"filter": {
736-
"param": "param_33"
736+
"param": "param_35"
737737
}
738738
},
739739
{
@@ -894,12 +894,12 @@
894894
},
895895
{
896896
"filter": {
897-
"param": "param_33"
897+
"param": "param_35"
898898
}
899899
},
900900
{
901901
"filter": {
902-
"param": "param_33"
902+
"param": "param_35"
903903
}
904904
},
905905
{
@@ -1061,12 +1061,12 @@
10611061
},
10621062
{
10631063
"filter": {
1064-
"param": "param_33"
1064+
"param": "param_35"
10651065
}
10661066
},
10671067
{
10681068
"filter": {
1069-
"param": "param_33"
1069+
"param": "param_35"
10701070
}
10711071
},
10721072
{
@@ -1236,12 +1236,12 @@
12361236
},
12371237
{
12381238
"filter": {
1239-
"param": "param_33"
1239+
"param": "param_35"
12401240
}
12411241
},
12421242
{
12431243
"filter": {
1244-
"param": "param_33"
1244+
"param": "param_35"
12451245
}
12461246
},
12471247
{
@@ -1381,12 +1381,12 @@
13811381
},
13821382
{
13831383
"filter": {
1384-
"param": "param_33"
1384+
"param": "param_35"
13851385
}
13861386
},
13871387
{
13881388
"filter": {
1389-
"param": "param_33"
1389+
"param": "param_35"
13901390
}
13911391
},
13921392
{
@@ -1470,7 +1470,7 @@
14701470
"condition": {
14711471
"test": {
14721472
"not": {
1473-
"param": "param_34"
1473+
"param": "param_36"
14741474
}
14751475
},
14761476
"value": "#3A3A3A"
@@ -1576,12 +1576,12 @@
15761576
},
15771577
{
15781578
"filter": {
1579-
"param": "param_33"
1579+
"param": "param_35"
15801580
}
15811581
},
15821582
{
15831583
"filter": {
1584-
"param": "param_33"
1584+
"param": "param_35"
15851585
}
15861586
},
15871587
{
@@ -1650,7 +1650,7 @@
16501650
"condition": {
16511651
"test": {
16521652
"not": {
1653-
"param": "param_34"
1653+
"param": "param_36"
16541654
}
16551655
},
16561656
"value": "#3A3A3A"
@@ -1751,12 +1751,12 @@
17511751
},
17521752
{
17531753
"filter": {
1754-
"param": "param_33"
1754+
"param": "param_35"
17551755
}
17561756
},
17571757
{
17581758
"filter": {
1759-
"param": "param_33"
1759+
"param": "param_35"
17601760
}
17611761
},
17621762
{

0 commit comments

Comments
 (0)