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

invert_colors fails for some image formats #421

Closed
zzril opened this issue Jul 6, 2023 · 2 comments
Closed

invert_colors fails for some image formats #421

zzril opened this issue Jul 6, 2023 · 2 comments
Labels
released Included in a release

Comments

@zzril
Copy link
Contributor

zzril commented Jul 6, 2023

Describe the bug

The invert_colors method for images internally uses ImageOps.invert(...):
src/safeds/data/image/containers/_image.py:

    def invert_colors(self) -> Image:
        """
        Return the image with inverted colors.


        Returns
        -------
        result : Image
            The image with inverted colors.
        """
        image_copy = copy.deepcopy(self)
        image_copy._image = ImageOps.invert(image_copy._image)
        return image_copy

This ImageOps.invert(...) method can fail with an obscure OSError for certain, non-RGB image formats.
For example, it fails for this image that is currently used in the image documentation.

To Reproduce

Checkout the 373-tutorial-for-image-processing branch, first commit (a3c1b1d) and run the 10th cell. (Or run everything, either in the notebook or by serving the documentation with poetry run mkdocs serve).

Expected behavior

If possible, the invert_colors method should never fail.

Stackoverflow has some tips on how to achieve that with ImageOps and non-RGB formats.

For cases where it is not possible to find a workaround, the internal error from ImageOps should be catched and a more user-friendly error should be thrown.

@zzril zzril added the bug 🪲 Something isn't working label Jul 6, 2023
@github-project-automation github-project-automation bot moved this to Backlog in Library Jul 6, 2023
@zzril zzril linked a pull request Jul 6, 2023 that will close this issue
@zzril
Copy link
Contributor Author

zzril commented Jul 6, 2023

This issue currently has a draft PR (#422) linked, but no assignee. This is intentional.
The PR was created because the commit that fixes the error mentioned in this issue was needed elsewhere already.

Feel free to take over the PR and finish it.

@zzril zzril added cleanup 🧹 Refactorings and other tasks that improve the code and removed bug 🪲 Something isn't working cleanup 🧹 Refactorings and other tasks that improve the code labels Jul 12, 2023
@zzril
Copy link
Contributor Author

zzril commented Jul 12, 2023

The bug has been fixed in #419. Images are now converted to RGB format before inverting them.

This might mean that inverting an image twice loses some quality compared to the original image. But we do not seem to test for this, and it's in general a pretty common thing for image operations (example: resize to a smaller size, then back to original).

After having a closer look into the documentation, I also don't see any internal exceptions that could be raised when calling the PIL's convert method, so there's nothing more to do here.

@zzril zzril closed this as completed Jul 12, 2023
@github-project-automation github-project-automation bot moved this from Backlog to ✔️ Done in Library Jul 12, 2023
@Marsmaennchen221 Marsmaennchen221 added the released Included in a release label Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Included in a release
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants