Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Adding an equals method to colormaps#20227
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| self._init() | ||
| if not other._isinit: | ||
| other._init() | ||
| return np.array_equal(self._lut, other._lut) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
colormaps know their own name, is that something we want to also check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, IMHO we should. As we use it now, the name is an integral part of the colormap. Otherwise you can get funny behavior, e.g. with the new colormap registry
cmap in colormaps.values() # Truecmap.name in colormaps.keys() # FalseAnd similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't have a strong opinion either way. I was viewing the== to mean "will these colormaps look the same". If cmocean defines a "Blues1" colormap and it looks suspiciously similar to MPL's "Blues", I may want to check for that equality and not name equality. I do see TIm's point though, so I am fine adding it if that is what is wanted.
I suppose we should also addcolorbar_extends too in case someone changed that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I see the point in "will these colormaps look the same", but as long as we haveColormap.name I think it has to be taken into account for__eq__. You could introduce aColormap.is_equivalent() for "look the same". OTOH I don't think there is a strong reason a colormap has to know its name, but removing that is hard to justify as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
OK, I added in the comparisons forname andcolorbar_extend. Also added tests for those two situations.
This adds an __eq__ method to Colormap to test the lookuptables for equality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Not sure when I would use this, but it looks right to me ;-)
I do not think this is a big enough feature to warrant a whats new. |
PR Summary
This adds an
__eq__method to Colormap to test the lookuptables for equality. This will help for testing whether colormaps are the same even if they aren't the same object.
plt.get_cmap("Blues") == plt.get_cmap("Blues")even if they aren't the same object anymore.Taking just this portion out of theclosed#16943.
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).