colour-science / colour

Colour Science for Python
https://www.colour-science.org
BSD 3-Clause "New" or "Revised" License
2.08k stars 259 forks source link

[FEATURE]: Add `__eq__` magic method to `RGB_Colourspace` class #1158

Open MrLixm opened 1 year ago

MrLixm commented 1 year ago

Description

Hello, I was wondering if there is any good reason why RGB_Colourspace class doesn't implement an __eq__ method to allow instance comparison like :

colorspace_a = colour.RGB_COLOURSPACES["sRGB"]
colorspace_b = colour.RGB_Colourspace("sRGB",  [[ 0.64,  0.33],[ 0.3 ,  0.6 ], [ 0.15,  0.06]], [ 0.3127,  0.329 ]],...)
assert colorspace_a == colorspace_b 

Cheers. Liam.

KelSolaar commented 1 year ago

No good reason, just did not have the need! That would be an exact equality too so 0.6400000001, 0.33 would not work.

nick-shaw commented 1 year ago

It would also raise the question of whether you would want a version of a colourspace which used a derived NPM to be considered equal to one with a "per spec" one.

MrLixm commented 1 year ago

It would also raise the question of whether you would want a version of a colourspace which used a derived NPM to be considered equal to one with a "per spec" one.

In that case I would say no. As thomas mentioned for me it would be a straight equal comparison of the class attributes :


import numpy

class RGB_Colourspace:
    def __eq__(self, other):
        if isinstance(other, self.__class__):
            return (
                self.name == other.name and
                numpy.array_equal(self.primaries, other.primaries) and
                numpy.array_equal(self.whitepoint, other.whitepoint) and
                self.whitepoint_name == other.whitepoint_name and
                numpy.array_equal(self.matrix_RGB_to_XYZ, other.matrix_RGB_to_XYZ) and
                numpy.array_equal(self.matrix_XYZ_to_RGB, other.matrix_XYZ_to_RGB) and
                self.cctf_encoding == other.cctf_encoding and
                self.cctf_decoding == other.cctf_decoding and
                self.use_derived_matrix_RGB_to_XYZ == other.use_derived_matrix_RGB_to_XYZ and
                self.use_derived_matrix_XYZ_to_RGB == other.use_derived_matrix_XYZ_to_RGB
            )
        return False

Without this I think we would also have the issue with cctf function where it would be impossible to know if they produce the same result. So we just check if they are the same object.

This make the use of eq pretty limited as just a way to check for example if 2 instances are actually the same, like one you would get with RGB_Colourspace().copy(), but still useful for that kind of case.

jamesmyatt commented 1 year ago

This would be free if RGB_Colourspace (and similar) were reimplemented using attrs or dataclass or similar. Would also reduce a lot of repeated and boilerplate code.

KelSolaar commented 1 year ago

That would not be a bad idea but we have quite a bit of logic on the properties, not easy.