镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

API changes for Resampling modes #6537

Closed

Description

@99991

With Pillow 9.2.0, I get the following deprecation notice:

DeprecationWarning: BILINEAR is deprecated and will be removed in Pillow 10 (2023-07-01). Use Resampling.BILINEAR instead.

Is it really necessary to introduce this breaking API change? Can the old names not be kept for backwards compatibility? Thousands of repositories will need to update their code, wasting countless man-hours.

You can get a rough idea on how many repositories are affected from the following two websites, which search a subset of GitHub repositories:

from PIL import Image
img = Image.new("RGB", (5, 5))
img.resize((7, 7), Image.BILINEAR)

Activity

  1. radarhere commented on Aug 25, 2022

    @radarhere
    Member

    To provide context, the deprecations being discussed were added in #5954. The intention was not to make an arbitrary change, but to make use of a newish Python language feature, enums.

    #5954 (comment)

    Do we keep the old constants "forever"?

    Or do we want to deprecate the old constants and remove them in Pillow 10 (15 months' deprecation period)?

    They were first deprecated in Pillow 9.1.0 - https://pillow.readthedocs.io/en/stable/releasenotes/9.1.0.html#deprecations

  2. radarhere commented on Aug 26, 2022

    @radarhere
    Member

    I think in the scheme of things, this is a very minor change for users to make when upgrading to a new version. In Javascript programming, I'm used to finding old npm packages abandoned and replaced with new ones, and in iOS programming, I'd be surprised if Apple went a year without changing any constants.

    If your argument is that Pillow is extremely widely used, I'll try and use NumPy as a point of reference. That is another Python library, downloaded twice as much as Pillow. In NumPy 1.20.0, they deprecated a series of aliases.

    https://grep.app/search?q=np.int seems to show that at least 20 times the amount of code is affected by that compared to https://grep.app/search?q=Image.BILINEAR.

    I make this comment not in the interest of taking a stance, but in the interest of giving you a quick response.

  3. 99991 commented on Aug 26, 2022

    @99991
    Author

    In NumPy 1.20.0, they deprecated a series of aliases.

    I believe that the argument for deprecating inbuilt types in NumPy is a bit stronger since it was know for a long time to be an actual problem numpy/numpy#6103

    The intention was not to make an arbitrary change, but to make use of a newish Python language feature, enums.

    The usage of a new Python language feature does not sound like a very convincing argument to me. Also, it is not an argument for breaking backwards compatibility.

    https://grep.app/search?q=np.int seems to show that at least 20 times the amount of code is affected by that compared to https://grep.app/search?q=Image.BILINEAR.

    Your query also includes the non-deprecated types like np.int_ or np.int32. If you exclude those with a regular expression like https://grep.app/search?q=np%5C.int%5B%5E0-9%5Ec%5Ep%5D&regexp=true there are 10 times fewer hits than https://grep.app/search?q=Image.BILINEAR

  4. added this to the 10.0.0 milestone on Aug 27, 2022
  5. dynobo commented on Sep 3, 2022

    @dynobo

    While I agree with the change in general, may I ask: What were the reasons to only bump pillow's minor version? Wouldn't, at least according to semver, a breaking API change of such a widely used feature would justify a new major release?
    Oh, scrape that, it was just introduced a deprecation warning so far. Everything good ;-)

  6. radarhere commented on Sep 3, 2022

    @radarhere
    Member

    The deprecation warning may have appeared in a minor version, but the removal of the original values won't happen until the next major version.

  7. changed the title [-]Unnecessary API changes for Resampling modes[/-] [+]API changes for Resampling modes[/+] on Sep 24, 2022
  8. hugovk commented on Oct 18, 2022

    @hugovk
    Member

    I'm having second thoughts about this API change.

    While enums are useful for this, and would be great for new things, I think this might be a bit too disruptive given the gains.

    It would be good to avoid a situation like the PILLOW_VERSION change as mentioned in #6614 (comment).

    What are other's thoughts?

    (As an aside, I wouldn't be averse to re-adding PILLOW_VERSION, it's just a constant; for example, it could be commented as deprecated with no warnings.)

  9. radarhere commented on Oct 19, 2022

    @radarhere
    Member

    PILLOW_VERSION was deprecated in 5.2.0, removed in 7.0.0, restored as deprecated in 7.1.0 and removed again in 9.0.0. Fully restoring it may well help people on a practical level, but as an end user, I would feel very unsure about whether PILLOW_VERSION will be present or not in any future version of Pillow.

    I don't have strong feeling on what the implementation is, only a preference that we aim to be consistent - if a decision is made, that we follow through. For a different example, I'm reluctant about #5309 because we previously told users to use close() to close files, but that issue would like to prevent that behaviour. I think clear communication and expectations are more fundamental than backwards compatibility.

  10. hugovk commented on Oct 24, 2022

    @hugovk
    Member

    PILLOW_VERSION was deprecated in 5.2.0, removed in 7.0.0, restored as deprecated in 7.1.0 and removed again in 9.0.0. Fully restoring it may well help people on a practical level, but as an end user, I would feel very unsure about whether PILLOW_VERSION will be present or not in any future version of Pillow.

    I think that would be fine: the message is end users should not use PILLOW_VERSION. We'd comment it as deprecated, no removal date listed. Anyone accessing PILLOW_VERSION via legacy code (often as a dependency as a dependency) won't know either way. Anyone writing new code who looks it up and sees the comment should use __version__ instead. But if they still use (e.g. by using autocomplete), it doesn't really matter. It's just a single constant, very little cost to leave it there than worry about removal.

    I don't have strong feeling on what the implementation is, only a preference that we aim to be consistent - if a decision is made, that we follow through. For a different example, I'm reluctant about #5309 because we previously told users to use close() to close files, but that issue would like to prevent that behaviour. I think clear communication and expectations are more fundamental than backwards compatibility.

    I think we can change our minds, but should do so with clear communication. We need to balance cost/benefits of BC breaks, and it would be better to do so before release (both as PILLOW_VERSION taught us).

    Here's a PR to revert in case we want to go ahead: #6684

  11. hugovk commented on Oct 25, 2022

    @hugovk
    Member

    I closed PR #6684, I somehow had glossed over that the enums were already released in 9.1.0, so this itself would be a breaking change.

    Some other ideas we're considering:

    • Keep the enums and deprecated ints
    • Keep the enums and deprecated ints, but extend deprecation period (e.g. from v10/July 2023 to v11/Oct 2024, or even without advertising an end date)
    • Provide API via both ints and enums, remove int deprecations
  12. radarhere commented on Dec 27, 2022

    @radarhere
    Member

    I've created PR #6830 as a suggestion for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions