Add type hints for ImageCms #7913
Conversation
| nitpick_ignore = [ | ||
| # sphinx does not understand `typing.Literal[-1]` | ||
| ("py:obj", "typing.Literal[-1, 1]"), | ||
| ] |
There was a problem hiding this comment.
This will be fixed in a future release of sphinx: sphinx-doc/sphinx#11904
| /* Ready object types */ | ||
| PyType_Ready(&CmsProfile_Type); | ||
| PyType_Ready(&CmsTransform_Type); | ||
|
|
||
| Py_INCREF(&CmsProfile_Type); | ||
| PyModule_AddObject(m, "CmsProfile", (PyObject *)&CmsProfile_Type); | ||
|
|
||
| Py_INCREF(&CmsTransform_Type); | ||
| PyModule_AddObject(m, "CmsTransform", (PyObject *)&CmsTransform_Type); |
There was a problem hiding this comment.
It seemed odd to me to expose one of the classes but not the other.
| @@ -1511,15 +1511,16 @@ setup_module(PyObject *m) { | |||
| PyObject *v; | |||
| int vn; | |||
|
|
|||
| CmsProfile_Type.tp_new = PyType_GenericNew; | |||
There was a problem hiding this comment.
This is not needed (and incorrect! should call core.profile_open or core.profile_frombytes instead).
I added new tests for this being unset, but on PyPy this is the default value.
src/PIL/ImageCms.py
Outdated
| @@ -787,14 +797,17 @@ def createProfile(colorSpace, colorTemp=-1): | |||
| except (TypeError, ValueError) as e: | |||
| msg = f'Color temperature must be numeric, "{colorTemp}" not valid' | |||
| raise PyCMSError(msg) from e | |||
| else: | |||
| # colorTemp is unused if colorSpace != "LAB" | |||
| colorTemp = 0.0 | |||
There was a problem hiding this comment.
This is minor, but considering it's unused, what's the advantage of setting it to zero?
There was a problem hiding this comment.
The value is passed to PyArg_ParseTuple with the d format. Based on the documentation, my understanding was that this required a float object, not a SupportsFloat object. However, testing with Python 3.11, it seems that a SupportsFloat object is actually accepted, so I'll revert this part of the change.
(Testing with SupportsInt I do get the expected TypeError, so at least that part matches the documentation.)
src/PIL/ImageCms.py
Outdated
| outMode: str, | ||
| renderingIntent: Intent = Intent.PERCEPTUAL, | ||
| proofRenderingIntent: Intent = Intent.ABSOLUTE_COLORIMETRIC, | ||
| flags: Flags | int = Flags.SOFTPROOFING, |
There was a problem hiding this comment.
Wouldn't our discussion in #7671 (comment) about favouring enums over int also apply to the IntFlag of Flags, meaning this could just be flags: Flags?
There was a problem hiding this comment.
Yep, you're right. IIRC I might have added int to support the deprecated ImageCms.FLAGS, but that does not have type hints and should not be used anyway.
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
|
Thanks! |
Changes proposed in this pull request:
ImageCmsandImageCms.core;ImageCms.core.profile_tobytes- this cannot be called via the non-core functions thanks to a type check inImageCms.ImageCmsProfile().ImageCms.rstto match the new ones in_imagingcms.pyi.