-
-
Notifications
You must be signed in to change notification settings - Fork 160
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
Allow all color types mask.from_threshold and fix typing for PixelArray methods #3164
Allow all color types mask.from_threshold and fix typing for PixelArray methods #3164
Conversation
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.
As a matter of personal preference, I would like to have consistent handling within pixelarray
for color values. As it is sort of documented at the module level, it would make sense to keep that behaviour everywhere in the module. Therefore I think this case is a "stubs should be fixed" thing
The mask change on the other hand, I am fine with.
@ankith26 I had actually forgot of this, now it should be ready |
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.
LGTM, thanks for the PR 🎉
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.
Looks good to me!
I understand why slice-assignment of pixelarrays limits the allowed types, but this was likely not intended for the
replace()
andextract()
methods, as they were typehinted withColorValue
but don't currently allow all color values.This PR adds a C function specifically for those methods, making the typehints true.
The same thing applies to
mask.from_threshold
.If you think they should not accept all color types, I can change the PR to change the typehints instead, of course.