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

Removed unused TiffImagePlugin IFD_LEGACY_API #8355

Merged
merged 1 commit into from
Sep 8, 2024

Conversation

radarhere
Copy link
Member

This value is not used anywhere.

IFD_LEGACY_API = True

@radarhere radarhere changed the title Removed unused IFD_LEGACY_API Removed unused TiffImagePlugin IFD_LEGACY_API Sep 6, 2024
@hugovk
Copy link
Member

hugovk commented Sep 6, 2024

Searching the top 8k PyPI projects only shows use in Pillow and type checkers:

python3 ~/github/misc/cpython/search_pypi_top.py -q . IFD_LEGACY_API
./pytype-2024.4.11.tar.gz: pytype-2024.4.11/pytype/typeshed/stubs/Pillow/PIL/TiffImagePlugin.pyi: IFD_LEGACY_API: bool
./pillow-10.3.0.tar.gz: pillow-10.3.0/src/PIL/TiffImagePlugin.py: IFD_LEGACY_API = True
./pillow-10.3.0.tar.gz: pillow-10.3.0/src/PIL/TiffImagePlugin.py: # undone -- switch this pointer when IFD_LEGACY_API == False
./pyre-check-0.9.22.tar.gz: pyre_check-0.9.22/typeshed/stubs/Pillow/PIL/TiffImagePlugin.pyi: IFD_LEGACY_API: bool
./types-Pillow-10.2.0.20240520.tar.gz: types-Pillow-10.2.0.20240520/PIL-stubs/TiffImagePlugin.pyi: IFD_LEGACY_API: bool

Time: 0:00:29.920569
Found 5 matching lines in 4 projects

Likewise with grep.app:

https://grep.app/search?q=IFD_LEGACY_API&case=true

And GitHub, where it's also in some AST symbol tables, but they look like dumps of specific versions:

https://github.com/search?q=IFD_LEGACY_API&ref=opensearch&type=code&p=1

Technically this is a breaking change, but I think we can make an exception to the deprecation period, and go straight to removal in a major version. Let's also document it in the release notes and removals.

Comment on lines 72 to 75
TiffImagePlugin IFD_LEGACY_API
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

TODO
An unused setting, ``TiffImagePlugin.IFD_LEGACY_API``, has been removed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this under "Backwards Incompatible Changes"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -1125,7 +1124,6 @@ def __getitem__(self, tag: int) -> Any:
return val


# undone -- switch this pointer when IFD_LEGACY_API == False
ImageFileDirectory = ImageFileDirectory_v1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there still plans to switch this in the future?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were something like 3 stages to the tiff IFD modernization, and I got through stage 1, years back now. Something like it is going to be required if we're going to do robust tiff tags, but it's not on my list anymore.

Copy link
Member Author

@radarhere radarhere Sep 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've restored the comment that it is 'undone' to 'switch this pointer'

@radarhere radarhere merged commit 579a5a2 into python-pillow:main Sep 8, 2024
47 of 48 checks passed
@radarhere radarhere deleted the unused_tiff branch September 8, 2024 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants