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

Doesn't work with JPGs #6

Open
sionleroux opened this issue Mar 2, 2023 · 4 comments
Open

Doesn't work with JPGs #6

sionleroux opened this issue Mar 2, 2023 · 4 comments

Comments

@sionleroux
Copy link
Contributor

Hi, the README says:

This tool supports only raster graphics.

All the examples show using PNGs but I thought I'd try a JPG and it crashes the program:

hexsticker dp.jpg 
INFO:hexsticker.create:Writing output to 'dp-sticker.jpg'
Traceback (most recent call last):
  File "/usr/lib/python3.10/site-packages/PIL/JpegImagePlugin.py", line 643, in _save
    rawmode = RAWMODE[im.mode]
KeyError: 'RGBA'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/lib/python3.10/site-packages/hexsticker/create.py", line 206, in create_hexsticker
    img.save(output_file, output_file_type.upper())
  File "/usr/lib/python3.10/site-packages/PIL/Image.py", line 2431, in save
    save_handler(self, fp, filename)
  File "/usr/lib/python3.10/site-packages/PIL/JpegImagePlugin.py", line 646, in _save
    raise OSError(msg) from e
OSError: cannot write mode RGBA as JPEG

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/bin/hexsticker", line 33, in <module>
    sys.exit(load_entry_point('hexsticker==1.2.0', 'console_scripts', 'hexsticker')())
  File "/usr/lib/python3.10/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/usr/lib/python3.10/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/usr/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/lib/python3.10/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/usr/lib/python3.10/site-packages/hexsticker/cli.py", line 69, in hexsticker
    create_hexsticker(**options)
  File "/usr/lib/python3.10/site-packages/hexsticker/create.py", line 208, in create_hexsticker
    raise exceptions.SaveError(f"Failed to save resulting image: {str(exc)}") from exc
hexsticker.exception.SaveError: Failed to save resulting image: cannot write mode RGBA as JPEG

Have I made a mistake, e.g. missed installing a library? Or does the program not support JPEG? I'd guess the latter because the stack trace mentions saving RGBA as JPEG, and I suppose the A in RGBA is because of the transparent background after the hexagon is cut out and JPEGs don't support transparency. Maybe it would be good to add a file type check at the beginning of the program to make sure the user provides an input filetype that will support transparency in the output?

@fridex
Copy link
Owner

fridex commented Mar 2, 2023

Hi @sinisterstuf, thanks for the report. Yes, JPEG is not supported. +1 on the filetype check. Would you mind submitting a PR for this? Thank you.

@sionleroux
Copy link
Contributor Author

Sure, I'm pretty unfamiliar with any of these libraries but I'll give it a go!

@sionleroux
Copy link
Contributor Author

Ok I checked the code a bit today and this crash is caused by this commit a0a9869 which changed the image mode from RGB to RGBA seemingly without consideration for the fact that only 3 out of 6 of the supported filetypes can handle transparency. I think it would be lame to revert this to its previous value because having transparent backgrounds is useful when possible.

Instead I intend to check whether the output file format supports transparency or not, and convert it to non-transparent RGB if necessary. This will involve some changes because it seems right now we only check the filetypes are valid for the input file. So:

  • Add a 2nd list of file types for only those that support transparency
  • Extract the filetype checking code to a smaller function (although maybe it's enough to add a condition inside the extension checking function and use that instead)
  • Use RGB if the file format doesn't support transparency
  • Maybe raise an exception if the input file would support transparency but the output file has been specified explicitly with -o and it does not support transparency, haven't decided yet if this is better to force it to non-transparent
  • Change the default background colour from transparent red to transparent white

@fridex
Copy link
Owner

fridex commented Mar 5, 2023

What about providing an option for this so users can control behaviour and raise an exception for unsupported combinations?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants