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

Image.register_open called multiple times #6916

Closed
bigcat88 opened this issue Jan 28, 2023 · 2 comments · Fixed by #6917
Closed

Image.register_open called multiple times #6916

bigcat88 opened this issue Jan 28, 2023 · 2 comments · Fixed by #6917

Comments

@bigcat88
Copy link
Contributor

What did you do?

Calling Image.register_open multiply times should not lead to growth of Image.ID list.

What did you expect to happen?

After second call to Image.register_open Image.ID should not contain similar values.

What are your OS, Python and Pillow versions?

  • OS: not OS dependent
  • Python: not python version dependent
  • Pillow: 9.4.0

Maybe it is invalid usage, I know that applications should not call Image.register_open, but this very small problem present.

def register_open(id, factory, accept=None):
    """
    Register an image file plugin.  This function should not be used
    in application code.

    :param id: An image format identifier.
    :param factory: An image file factory method.
    :param accept: An optional function that can be used to quickly
       reject images having another format.
    """
    id = id.upper()
    ID.append(id)  # growth of the list here.
    OPEN[id] = factory, accept

Image.ID used only in a few places:

  1. PIL.Image.open
    if formats is None:
        formats = ID
  1. PIL.FitsStubImagePlugin
    # Override FitsImagePlugin with this handler
    # for backwards compatibility
    try:
        Image.ID.remove(FITSStubImageFile.format)
    except ValueError:
        pass

    Image.register_open(
        FITSStubImageFile.format, FITSStubImageFile, FitsImagePlugin._accept
    )
  1. PIL.features
        for i in sorted(Image.ID):
            line = f"{i}"
            if i in Image.MIME:
                line = f"{line} {Image.MIME[i]}"
            print(line, file=out)

Probably in 1 and 3 places this can be replaced with OPEN.keys() that contain the same values, if I understand code correct, and in place with number 2 just remove Image.ID.remove lines.
These changes should not affect performance.

@radarhere radarhere changed the title Image.register_open called multiply times Image.register_open called multiple times Jan 28, 2023
@radarhere
Copy link
Member

To be clear - you didn't actually have code that you'd written that you found calling Image.register_open multiple times, right? This is just a theoretical concern?

I'm reluctant to make ID redundant by replacing it with OPEN.keys(), since ID is part of the public API.

I've created PR #6917 to simply not append existing ids to ID.

@bigcat88
Copy link
Contributor Author

bigcat88 commented Jan 28, 2023

To be clear - you didn't actually have code that you'd written that you found calling Image.register_open multiple times, right? This is just a theoretical concern?

Yes, it is more theoretical, you are right.

I've created PR #6917 to simply not append existing ids to ID.

Thanks.

@mergify mergify bot closed this as completed in #6917 Jan 28, 2023
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

Successfully merging a pull request may close this issue.

2 participants