diff --git a/CHANGELOG.md b/CHANGELOG.md index 126ff280b4..022fe65b1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,9 +5,13 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased ### Fixed +- Fix `ColorValidator` silently accepting invalid colors in 2D numpy arrays instead of raising `ValueError` [[#5576](https://github.com/plotly/plotly.py/pull/5576)] - Fix issue where user-specified `color_continuous_scale` was ignored when template had `autocolorscale=True` [[#5439](https://github.com/plotly/plotly.py/pull/5439)], with thanks to @antonymilne for the contribution! - Update tests to be compatible with numpy 2.4 [[#5522](https://github.com/plotly/plotly.py/pull/5522)], with thanks to @thunze for the contribution! +### Performance +- Optimize `ColorValidator` array validation: up to 3x faster on numpy arrays, 2x on lists, 14x on scalars [[#5576](https://github.com/plotly/plotly.py/pull/5576)] + ### Updated - The `__eq__` method for `graph_objects` classes now returns `NotImplemented` to give the other operand an opportunity to handle the comparison [[#5547](https://github.com/plotly/plotly.py/pull/5547)], with thanks to @RazerM for the contribution! diff --git a/_plotly_utils/basevalidators.py b/_plotly_utils/basevalidators.py index d79c5adff5..8c8e21a4dc 100644 --- a/_plotly_utils/basevalidators.py +++ b/_plotly_utils/basevalidators.py @@ -1165,156 +1165,158 @@ class ColorValidator(BaseValidator): re_rgb_etc = re.compile(r"(rgb|hsl|hsv)a?\([\d.]+%?(,[\d.]+%?){2,3}\)") re_ddk = re.compile(r"var\(\-\-.*\)") - named_colors = [ - "aliceblue", - "antiquewhite", - "aqua", - "aquamarine", - "azure", - "beige", - "bisque", - "black", - "blanchedalmond", - "blue", - "blueviolet", - "brown", - "burlywood", - "cadetblue", - "chartreuse", - "chocolate", - "coral", - "cornflowerblue", - "cornsilk", - "crimson", - "cyan", - "darkblue", - "darkcyan", - "darkgoldenrod", - "darkgray", - "darkgrey", - "darkgreen", - "darkkhaki", - "darkmagenta", - "darkolivegreen", - "darkorange", - "darkorchid", - "darkred", - "darksalmon", - "darkseagreen", - "darkslateblue", - "darkslategray", - "darkslategrey", - "darkturquoise", - "darkviolet", - "deeppink", - "deepskyblue", - "dimgray", - "dimgrey", - "dodgerblue", - "firebrick", - "floralwhite", - "forestgreen", - "fuchsia", - "gainsboro", - "ghostwhite", - "gold", - "goldenrod", - "gray", - "grey", - "green", - "greenyellow", - "honeydew", - "hotpink", - "indianred", - "indigo", - "ivory", - "khaki", - "lavender", - "lavenderblush", - "lawngreen", - "lemonchiffon", - "lightblue", - "lightcoral", - "lightcyan", - "lightgoldenrodyellow", - "lightgray", - "lightgrey", - "lightgreen", - "lightpink", - "lightsalmon", - "lightseagreen", - "lightskyblue", - "lightslategray", - "lightslategrey", - "lightsteelblue", - "lightyellow", - "lime", - "limegreen", - "linen", - "magenta", - "maroon", - "mediumaquamarine", - "mediumblue", - "mediumorchid", - "mediumpurple", - "mediumseagreen", - "mediumslateblue", - "mediumspringgreen", - "mediumturquoise", - "mediumvioletred", - "midnightblue", - "mintcream", - "mistyrose", - "moccasin", - "navajowhite", - "navy", - "oldlace", - "olive", - "olivedrab", - "orange", - "orangered", - "orchid", - "palegoldenrod", - "palegreen", - "paleturquoise", - "palevioletred", - "papayawhip", - "peachpuff", - "peru", - "pink", - "plum", - "powderblue", - "purple", - "red", - "rosybrown", - "royalblue", - "rebeccapurple", - "saddlebrown", - "salmon", - "sandybrown", - "seagreen", - "seashell", - "sienna", - "silver", - "skyblue", - "slateblue", - "slategray", - "slategrey", - "snow", - "springgreen", - "steelblue", - "tan", - "teal", - "thistle", - "tomato", - "turquoise", - "violet", - "wheat", - "white", - "whitesmoke", - "yellow", - "yellowgreen", - ] + named_colors = frozenset( + [ + "aliceblue", + "antiquewhite", + "aqua", + "aquamarine", + "azure", + "beige", + "bisque", + "black", + "blanchedalmond", + "blue", + "blueviolet", + "brown", + "burlywood", + "cadetblue", + "chartreuse", + "chocolate", + "coral", + "cornflowerblue", + "cornsilk", + "crimson", + "cyan", + "darkblue", + "darkcyan", + "darkgoldenrod", + "darkgray", + "darkgrey", + "darkgreen", + "darkkhaki", + "darkmagenta", + "darkolivegreen", + "darkorange", + "darkorchid", + "darkred", + "darksalmon", + "darkseagreen", + "darkslateblue", + "darkslategray", + "darkslategrey", + "darkturquoise", + "darkviolet", + "deeppink", + "deepskyblue", + "dimgray", + "dimgrey", + "dodgerblue", + "firebrick", + "floralwhite", + "forestgreen", + "fuchsia", + "gainsboro", + "ghostwhite", + "gold", + "goldenrod", + "gray", + "grey", + "green", + "greenyellow", + "honeydew", + "hotpink", + "indianred", + "indigo", + "ivory", + "khaki", + "lavender", + "lavenderblush", + "lawngreen", + "lemonchiffon", + "lightblue", + "lightcoral", + "lightcyan", + "lightgoldenrodyellow", + "lightgray", + "lightgrey", + "lightgreen", + "lightpink", + "lightsalmon", + "lightseagreen", + "lightskyblue", + "lightslategray", + "lightslategrey", + "lightsteelblue", + "lightyellow", + "lime", + "limegreen", + "linen", + "magenta", + "maroon", + "mediumaquamarine", + "mediumblue", + "mediumorchid", + "mediumpurple", + "mediumseagreen", + "mediumslateblue", + "mediumspringgreen", + "mediumturquoise", + "mediumvioletred", + "midnightblue", + "mintcream", + "mistyrose", + "moccasin", + "navajowhite", + "navy", + "oldlace", + "olive", + "olivedrab", + "orange", + "orangered", + "orchid", + "palegoldenrod", + "palegreen", + "paleturquoise", + "palevioletred", + "papayawhip", + "peachpuff", + "peru", + "pink", + "plum", + "powderblue", + "purple", + "red", + "rosybrown", + "royalblue", + "rebeccapurple", + "saddlebrown", + "salmon", + "sandybrown", + "seagreen", + "seashell", + "sienna", + "silver", + "skyblue", + "slateblue", + "slategray", + "slategrey", + "snow", + "springgreen", + "steelblue", + "tan", + "teal", + "thistle", + "tomato", + "turquoise", + "violet", + "wheat", + "white", + "whitesmoke", + "yellow", + "yellowgreen", + ] + ) def __init__( self, plotly_name, parent_name, array_ok=False, colorscale_path=None, **kwargs @@ -1371,22 +1373,47 @@ def validate_coerce(self, v, should_raise=True): # All good pass else: - validated_v = [self.validate_coerce(e, should_raise=False) for e in v] - - invalid_els = self.find_invalid_els(v, validated_v) + # For 1-D numpy arrays, elements are scalars — call + # perform_validate_coerce directly to skip the per-element + # array-type dispatch in validate_coerce. + allow_number = self.numbers_allowed() + pvc = ColorValidator.perform_validate_coerce + validated_v = [] + invalid_els = [] + if v.ndim == 1: + for e in v: + ve = pvc(e, allow_number=allow_number) + validated_v.append(ve) + if ve is None: + invalid_els.append(e) + else: + for e in v: + ve = self.validate_coerce(e, should_raise=False) + validated_v.append(ve) + self.find_invalid_els(e, ve, invalid_els) if invalid_els and should_raise: self.raise_invalid_elements(invalid_els) # ### Check that elements have valid colors types ### - elif self.numbers_allowed() or invalid_els: + elif allow_number or invalid_els: v = copy_to_readonly_numpy_array(validated_v, kind="O") else: v = copy_to_readonly_numpy_array(validated_v, kind="U") elif self.array_ok and is_simple_array(v): - validated_v = [self.validate_coerce(e, should_raise=False) for e in v] - - invalid_els = self.find_invalid_els(v, validated_v) + allow_number = self.numbers_allowed() + pvc = ColorValidator.perform_validate_coerce + validated_v = [] + invalid_els = [] + for e in v: + if is_array(e): + ve = self.validate_coerce(e, should_raise=False) + self.find_invalid_els(e, ve, invalid_els) + else: + ve = pvc(e, allow_number=allow_number) + if ve is None: + invalid_els.append(e) + validated_v.append(ve) if invalid_els and should_raise: self.raise_invalid_elements(invalid_els) @@ -1402,7 +1429,7 @@ def validate_coerce(self, v, should_raise=True): return v - def find_invalid_els(self, orig, validated, invalid_els=None): + def find_invalid_els(self, orig, validated, invalid_els): """ Helper method to find invalid elements in orig array. Elements are invalid if their corresponding element in @@ -1410,9 +1437,6 @@ def find_invalid_els(self, orig, validated, invalid_els=None): This method handles deeply nested list structures """ - if invalid_els is None: - invalid_els = [] - for orig_el, validated_el in zip(orig, validated): if is_array(orig_el): self.find_invalid_els(orig_el, validated_el, invalid_els) @@ -1420,8 +1444,6 @@ def find_invalid_els(self, orig, validated, invalid_els=None): if validated_el is None: invalid_els.append(orig_el) - return invalid_els - def vc_scalar(self, v): """Helper to validate/coerce a scalar color""" return ColorValidator.perform_validate_coerce( @@ -1457,22 +1479,19 @@ def perform_validate_coerce(v, allow_number=None): # Remove spaces so regexes don't need to bother with them. v_normalized = v.replace(" ", "").lower() - # if ColorValidator.re_hex.fullmatch(v_normalized): - if fullmatch(ColorValidator.re_hex, v_normalized): + if ColorValidator.re_hex.fullmatch(v_normalized): # valid hex color (e.g. #f34ab3) return v - elif fullmatch(ColorValidator.re_rgb_etc, v_normalized): - # elif ColorValidator.re_rgb_etc.fullmatch(v_normalized): + elif ColorValidator.re_rgb_etc.fullmatch(v_normalized): # Valid rgb(a), hsl(a), hsv(a) color # (e.g. rgba(10, 234, 200, 50%) return v - elif fullmatch(ColorValidator.re_ddk, v_normalized): - # Valid var(--*) DDK theme variable, inspired by CSS syntax - # (e.g. var(--accent) ) - # DDK will crawl & eval var(-- colors for Graph theming - return v elif v_normalized in ColorValidator.named_colors: # Valid named color (e.g. 'coral') + # Checked before ddk regex since named colors are far more common + return v + elif ColorValidator.re_ddk.fullmatch(v_normalized): + # Valid var(--*) DDK theme variable return v else: # Not a valid color diff --git a/tests/test_plotly_utils/validators/test_color_validator.py b/tests/test_plotly_utils/validators/test_color_validator.py index c28c598321..4db7445fc6 100644 --- a/tests/test_plotly_utils/validators/test_color_validator.py +++ b/tests/test_plotly_utils/validators/test_color_validator.py @@ -241,6 +241,89 @@ def test_rejection_aok_colorscale(val, validator_aok_colorscale): # Test dynamic description logic +def test_acceptance_aok_none(validator_aok): + """None input should pass through unchanged (typed_array_spec path).""" + assert validator_aok.validate_coerce(None) is None + + +def test_acceptance_aok_typed_array_spec(validator_aok): + """Typed array spec dict should pass through unchanged.""" + spec = {"bdata": "AQID", "dtype": "i1"} + result = validator_aok.validate_coerce(spec) + assert result == spec + + +@pytest.mark.parametrize( + "val", + [ + np.array(["redd", "rgb(255, 0, 0)"]), + np.array(["bad_color"]), + ], +) +def test_rejection_aok_numpy_1d(val, validator_aok): + """Invalid colors in a 1D numpy array should raise.""" + with pytest.raises(ValueError) as validation_failure: + validator_aok.validate_coerce(val) + + assert "Invalid element(s)" in str(validation_failure.value) + + +def test_rejection_aok_numpy_1d_colorscale(validator_aok_colorscale): + """Invalid colors in a 1D numpy string array with numbers_allowed should raise.""" + val = np.array(["redd", "rgb(255, 0, 0)"]) + with pytest.raises(ValueError) as validation_failure: + validator_aok_colorscale.validate_coerce(val) + + assert "Invalid element(s)" in str(validation_failure.value) + + +def test_rejection_aok_nested_list_with_invalid(validator_aok): + """Nested list with invalid colors should raise, exercising find_invalid_els.""" + val = [["redd", "rgb(255, 0, 0)"], ["blue", "not_a_color"]] + with pytest.raises(ValueError) as validation_failure: + validator_aok.validate_coerce(val) + + assert "Invalid element(s)" in str(validation_failure.value) + + +def test_acceptance_aok_3d_nested_list(validator_aok): + """3-level nested list should validate, exercising recursive find_invalid_els.""" + val = [[["red", "blue"], ["green"]]] + result = validator_aok.validate_coerce(val) + assert validator_aok.present(result) == tuple(val) + + +def test_rejection_aok_3d_nested_list(validator_aok): + """3-level nested list with invalid colors should raise.""" + val = [[["redd", "blue"], ["green"]]] + with pytest.raises(ValueError) as validation_failure: + validator_aok.validate_coerce(val) + + assert "Invalid element(s)" in str(validation_failure.value) + + +@pytest.mark.parametrize( + "val", + [ + np.array([["redd", "rgb(255, 0, 0)"], ["blue", "not_a_color"]]), + np.array([["bad_color", "blue"]]), + ], +) +def test_rejection_aok_numpy_2d(val, validator_aok): + """Invalid colors in a 2D numpy array should raise.""" + with pytest.raises(ValueError) as validation_failure: + validator_aok.validate_coerce(val) + + assert "Invalid element(s)" in str(validation_failure.value) + + +def test_acceptance_aok_colorscale_numpy_numeric(validator_aok_colorscale): + """Numeric numpy array with numbers_allowed should pass through (numeric fast path).""" + val = np.array([0, 1, 2, 3]) + result = validator_aok_colorscale.validate_coerce(val) + assert np.array_equal(result, val) + + def test_description(validator): desc = validator.description() assert "A number that will be interpreted as a color" not in desc