-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Merged _MODE_CONV typ into ImageMode as typestr #6057
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
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.
I left some small, perhaps opinionated, comments. Looking good from my side otherwise.
src/PIL/Image.py
Outdated
| shape = (im.size[1], im.size[0]) | ||
| extra = len(m.bands) | ||
| if extra != 1: | ||
| shape += (extra,) |
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.
| shape = (im.size[1], im.size[0]) | |
| extra = len(m.bands) | |
| if extra != 1: | |
| shape += (extra,) | |
| shape = (im.size[1], im.size[0]) # im.size[::-1] | |
| shape = shape if len(m.bands) == 1 else (*shape, len(m.bands)) |
Not sure if you find this syntax neater. Both options look viable to me.
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.
It does seem to be a matter of personal preference, but shape = shape looks odd to me.
I have clarified the first line slightly by changing it to (im.height, im.width).
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.
that's fair.
|
Thanks both! |
Resolves #5990
#5990 (comment) pointed out that the second value in
_MODE_CONV,extra, is the number of bands in the mode. Rather than writing those out in_MODE_CONV, this could instead be derived from ImageMode.The user in the issue would also like the NumPy typestr from
_MODE_CONVavailable through a public API, and suggested ImageMode. From an organisational point of view for Pillow's code, that seems like a more logical location for that information, and it would be neater, since it then means that_MODE_CONVcan be removed. The NumPy typestr could then be accessed by a user throughImageMode.getmode(mode).typestr.