From ee4a6e5c4ca73e6d8f8b1381e1abfadc0ec880bb Mon Sep 17 00:00:00 2001 From: Isaac Muse Date: Fri, 15 Jul 2022 20:06:18 -0600 Subject: [PATCH] Remove naive space filtering (#210) --- coloraide/color.py | 64 +++++++++---------------- docs/src/dictionary/en-custom.txt | 1 + docs/src/markdown/about/changelog.md | 4 ++ docs/src/markdown/about/releases/1.0.md | 16 +++++++ docs/src/markdown/color.md | 20 -------- tests/test_api.py | 46 +++++------------- 6 files changed, 54 insertions(+), 97 deletions(-) diff --git a/coloraide/color.py b/coloraide/color.py index b5f825dc4..f4f4cf749 100644 --- a/coloraide/color.py +++ b/coloraide/color.py @@ -83,7 +83,7 @@ from .filters.w3c_filter_effects import Sepia, Brightness, Contrast, Saturate, Opacity, HueRotate, Grayscale, Invert from .filters.cvd import Protan, Deutan, Tritan from .types import Plugin -from typing import overload, Union, Sequence, Dict, List, Optional, Any, cast, Callable, Set, Tuple, Type, Mapping +from typing import overload, Union, Sequence, Dict, List, Optional, Any, cast, Callable, Tuple, Type, Mapping SUPPORTED_DE = ( DE76, DE94, DECMC, DE2000, DEHyAB, DEOK @@ -201,13 +201,11 @@ def __init__( color: ColorInput, data: Optional[VectorLike] = None, alpha: float = util.DEF_ALPHA, - *, - filters: Optional[Sequence[str]] = None, **kwargs: Any ) -> None: """Initialize.""" - self._space, self._coords = self._parse(color, data, alpha, filters=filters, **kwargs) + self._space, self._coords = self._parse(color, data, alpha, **kwargs) def __len__(self) -> int: """Get number of channels.""" @@ -261,29 +259,27 @@ def _parse( color: ColorInput, data: Optional[VectorLike] = None, alpha: float = util.DEF_ALPHA, - *, - filters: Optional[Sequence[str]] = None, **kwargs: Any ) -> Tuple[Type[Space], List[float]]: """Parse the color.""" - obj = None if isinstance(color, str): # Parse a color space name and coordinates if data is not None: s = color space_class = cls.CS_MAP.get(s) - if space_class and (not filters or s in filters): - num_channels = len(space_class.CHANNELS) - if len(data) < num_channels: - data = list(data) + [alg.NaN] * (num_channels - len(data)) - coords = [alg.clamp(float(v), *c.limit) for c, v in zipl(space_class.CHANNELS, data)] - coords.append(alg.clamp(float(alpha), *space_class.get_channel(-1).limit)) - obj = space_class, coords + if not space_class: + raise ValueError("'{}' is not a registered color space") + num_channels = len(space_class.CHANNELS) + if len(data) < num_channels: + data = list(data) + [alg.NaN] * (num_channels - len(data)) + coords = [alg.clamp(float(v), *c.limit) for c, v in zipl(space_class.CHANNELS, data)] + coords.append(alg.clamp(float(alpha), *space_class.get_channel(-1).limit)) + obj = space_class, coords # Parse a CSS string else: - m = cls._match(color, fullmatch=True, filters=filters) + m = cls._match(color, fullmatch=True) if m is None: raise ValueError("'{}' is not a valid color".format(color)) coords = [alg.clamp(float(v), *c.limit) for c, v in zipl(m[0].CHANNELS, m[1])] @@ -291,9 +287,10 @@ def _parse( obj = m[0], coords elif isinstance(color, Color): # Handle a color instance - if not filters or color.space() in filters: - space_class = cls.CS_MAP[color.space()] - obj = space_class, color[:] + space_class = cls.CS_MAP.get(color.space()) + if not space_class: + raise ValueError("'{}' is not a registered color space") + obj = space_class, color[:] elif isinstance(color, Mapping): # Handle a color dictionary space = color['space'] @@ -303,8 +300,6 @@ def _parse( else: raise TypeError("'{}' is an unrecognized type".format(type(color))) - if obj is None: - raise ValueError("Could not process the provided color") return obj @classmethod @@ -312,8 +307,7 @@ def _match( cls, string: str, start: int = 0, - fullmatch: bool = False, - filters: Optional[Sequence[str]] = None + fullmatch: bool = False ) -> Optional[Tuple[Type['Space'], Vector, float, int, int]]: """ Match a color in a buffer and return a color object. @@ -321,19 +315,13 @@ def _match( This must return the color space, not the Color object. """ - filter_set = set(filters) if filters is not None else set() # type: Set[str] - # Attempt color match m = parse.parse_color(string, cls.CS_MAP, start, fullmatch) if m is not None: - if not filter_set or m[0].NAME in filter_set: - return m[0], m[1][0], m[1][1], start, m[2] - return None + return m[0], m[1][0], m[1][1], start, m[2] # Attempt color space specific match for space, space_class in cls.CS_MAP.items(): - if filter_set and space not in filter_set: - continue m2 = space_class.match(string, start, fullmatch) if m2 is not None: return space_class, m2[0][0], m2[0][1], start, m2[1] @@ -344,13 +332,11 @@ def match( cls, string: str, start: int = 0, - fullmatch: bool = False, - *, - filters: Optional[Sequence[str]] = None + fullmatch: bool = False ) -> Optional[ColorMatch]: """Match color.""" - m = cls._match(string, start, fullmatch, filters=filters) + m = cls._match(string, start, fullmatch) if m is not None: return ColorMatch(cls(m[0].NAME, m[1], m[2]), m[3], m[4]) return None @@ -540,13 +526,11 @@ def new( color: ColorInput, data: Optional[VectorLike] = None, alpha: float = util.DEF_ALPHA, - *, - filters: Optional[Sequence[str]] = None, **kwargs: Any ) -> 'Color': """Create new color object.""" - return type(self)(color, data, alpha, filters=filters, **kwargs) + return type(self)(color, data, alpha, **kwargs) def clone(self) -> 'Color': """Clone.""" @@ -578,13 +562,11 @@ def mutate( color: ColorInput, data: Optional[VectorLike] = None, alpha: float = util.DEF_ALPHA, - *, - filters: Optional[Sequence[str]] = None, **kwargs: Any ) -> 'Color': """Mutate the current color to a new color.""" - self._space, self._coords = self._parse(color, data=data, alpha=alpha, filters=filters, **kwargs) + self._space, self._coords = self._parse(color, data=data, alpha=alpha, **kwargs) return self def update( @@ -592,14 +574,12 @@ def update( color: Union['Color', str, Mapping[str, Any]], data: Optional[VectorLike] = None, alpha: float = util.DEF_ALPHA, - *, - filters: Optional[Sequence[str]] = None, **kwargs: Any ) -> 'Color': """Update the existing color space with the provided color.""" space = self.space() - self._space, self._coords = self._parse(color, data=data, alpha=alpha, filters=filters, **kwargs) + self._space, self._coords = self._parse(color, data=data, alpha=alpha, **kwargs) if self._space.NAME != space: self.convert(space, in_place=True) return self diff --git a/docs/src/dictionary/en-custom.txt b/docs/src/dictionary/en-custom.txt index 72d1e7594..d16359b03 100644 --- a/docs/src/dictionary/en-custom.txt +++ b/docs/src/dictionary/en-custom.txt @@ -187,6 +187,7 @@ hz illuminant illuminants indexable +instantiation interpolator io ish diff --git a/docs/src/markdown/about/changelog.md b/docs/src/markdown/about/changelog.md index 960d7dc3e..561c1ef15 100644 --- a/docs/src/markdown/about/changelog.md +++ b/docs/src/markdown/about/changelog.md @@ -2,6 +2,10 @@ ## 1.0b2 +!!! warning "Breaking Changes" + 1.0b2 only introduces one more last breaking change that was forgotten in 1.0b1. + +- **BREAK**: Remove `filters` parameter on new class instantiation. - **NEW**: Added new migration guide to the documentation to help early adopters move to the 1.0 release. - **NEW**: Added HPLuv space described in the HSLuv spec. - **NEW**: Added new color spaces: ACES 2065-1, ACEScg, ACEScc, and ACEScct. diff --git a/docs/src/markdown/about/releases/1.0.md b/docs/src/markdown/about/releases/1.0.md index 938c56d10..a1b974335 100644 --- a/docs/src/markdown/about/releases/1.0.md +++ b/docs/src/markdown/about/releases/1.0.md @@ -227,3 +227,19 @@ the logic allowing us to even add a new interpolation method! ```playground Color.interpolate(['red', 'blue', 'green', 'orange'], method='bezier') ``` + +## Color Space Filters + +In the beginning, the `Color` space object was created with a naive filtering system. It added a little overhead, but +the real issue was the fact that it _only_ filtered inputs through `new`, `match`, and through normal instantiation. It +did _not_ filter through almost any other method that accepted inputs. It was decided to leave color filtering up to the +user. + +```playground +c = Color('display-p3', [1, 1, 0]) +try: + if c.space() not in ['srgb', 'hsl', 'hwb']: + raise ValueError('Invalid Color Space') +except ValueError as e: + print(e) +``` diff --git a/docs/src/markdown/color.md b/docs/src/markdown/color.md index 75cb0796e..67725e78e 100644 --- a/docs/src/markdown/color.md +++ b/docs/src/markdown/color.md @@ -91,18 +91,6 @@ color1 color1.new("blue") ``` -If desired, all creation methods can have a color space filter list passed in. The filter list will prevent an input -which specifies a color space found in our list to not be accepted. Using a filter will constrain inputs to only the -color spaces in the list. - -```playground -try: - Color("red", filters=["hsl"]) -except ValueError: - print('Not a valid color') -Color("hsl(130 30% 75%)", filters=["hsl"]) -``` - ## Random If you'd like to generate a random color, simply call `Color.random` with a given color space and one will be generated. @@ -277,14 +265,6 @@ later, we will match `#!color yellow` instead of `#!color red`. Color.match("red and yellow", start=8) ``` -Filtering unwanted color spaces is also available via the `filter` parameter, and is typically how creation methods -avoid parsing unwanted color spaces. - -```playground -Color.match("red and yellow", filters=["hsl"]) -Color.match("hsl(130 30% 75%)", filters=["hsl"]) -``` - A method to find all colors in a buffer is not currently provided as looping through all the color spaces and matching all potential colors on every character is not really efficient. Additionally, some buffers may require additional context that is not available to the match function. If such behavior is desired, it is recommended to apply some diff --git a/tests/test_api.py b/tests/test_api.py index 4e3350bc6..ca6546eec 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1,6 +1,6 @@ """Test miscellaneous API features.""" import unittest -from coloraide import Color, NaN +from coloraide import Color, ColorAll, NaN from . import util import math @@ -112,6 +112,16 @@ def test_bad_sytnax_input(self): with self.assertRaises(ValueError): Color("nope") + with self.assertRaises(ValueError): + Color("nope", [0, 0, 0]) + + def test_bad_class(self): + """Test bad class.""" + + c = ColorAll('hunter-lab', [0, 0, 0]) + with self.assertRaises(ValueError): + Color(c) + def test_bad_data_input(self): """Test bad data input.""" @@ -124,34 +134,6 @@ def test_missing_values(self): with self.assertRaises(ValueError): Color('color(srgb)') - def test_filtered_input(self): - """Test filtered input.""" - - self.assertTrue(isinstance(Color("red", filters=['srgb']), Color)) - with self.assertRaises(ValueError): - Color("hsl(20 100% 50%)", filters=['srgb']) - - def test_filtered_color_syntax_input(self): - """Test filtered input with color syntax.""" - - self.assertTrue(isinstance(Color("red", filters=['srgb']), Color)) - with self.assertRaises(ValueError): - Color("color(--hsl 20 100% 50%)", filters=['srgb']) - - def test_filtered_color_input(self): - """Test filtered Color input.""" - - self.assertTrue(isinstance(Color(Color("red"), filters=['srgb']), Color)) - with self.assertRaises(ValueError): - Color(Color("hsl(20 100% 50%)"), filters=['srgb']) - - def test_filtered_raw_input(self): - """Test filtered raw input.""" - - self.assertTrue(isinstance(Color(Color("srgb", [1, 1, 1]), filters=['srgb']), Color)) - with self.assertRaises(ValueError): - Color(Color("hsl", [20, 100, 50]), filters=['srgb']) - def test_missing_inputs(self): """Test missing inputs.""" @@ -400,12 +382,6 @@ def test_match_offset(self): self.assertEqual(obj.start, 21) self.assertEqual(obj.end, 35) - def test_match_filters(self): - """Test match with filters.""" - - self.assertIsNotNone(Color.match('lab(100% 0 0)')) - self.assertIsNone(Color.match('lab(100% 0 0)', filters=['srgb'])) - def test_mask_in_place(self): """Test masking "in place"."""