Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
MrLixm opened this issue May 28, 2023 · 5 comments
Open

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

MrLixm opened this issue May 28, 2023 · 5 comments

Comments

@MrLixm
Copy link
Contributor

MrLixm commented May 28, 2023

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.

@MrLixm MrLixm added the Feature label May 28, 2023
@KelSolaar KelSolaar added this to the v0.4.3 milestone May 29, 2023
@KelSolaar
Copy link
Member

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
Copy link
Contributor

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
Copy link
Contributor Author

MrLixm commented May 29, 2023

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
Copy link

jamesmyatt commented Jun 28, 2023

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
Copy link
Member

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

@KelSolaar KelSolaar modified the milestones: v0.4.3, v0.4.4 Aug 25, 2023
@KelSolaar KelSolaar modified the milestones: v0.4.4, v0.4.5 Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants