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

[dependency] update pillow pins #27409

Merged
merged 10 commits into from
Nov 22, 2023
Merged

[dependency] update pillow pins #27409

merged 10 commits into from
Nov 22, 2023

Conversation

ArthurZucker
Copy link
Collaborator

What does this PR do?

Following, High Severity CVE, trying to update the pinned versions

@ArthurZucker ArthurZucker changed the title update pillow pins [WIP] update pillow pins Nov 9, 2023
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Nov 9, 2023

The documentation is not available anymore as the PR was closed or merged.

setup.py Outdated Show resolved Hide resolved
src/transformers/dependency_versions_table.py Outdated Show resolved Hide resolved
@ArthurZucker ArthurZucker changed the title [WIP] update pillow pins [dependency] update pillow pins Nov 15, 2023
Copy link
Collaborator

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Change LGTM. Could you run slow tests for some of the vision models before merging? I'd suggest: CLIP, ViT, DETR, SAM

setup.py Outdated
@@ -95,7 +95,7 @@
# 1. all dependencies should be listed here with their version requirements if any
# 2. once modified, run: `make deps_table_update` to update src/transformers/dependency_versions_table.py
_deps = [
"Pillow<10.0.0",
"Pillow>=10.0.1,<=11.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why <=11.0?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll pin <=15.0, it's just to not allow whatever!
Thanks for the tips will run the slow model tests 😉

Copy link
Collaborator Author

@ArthurZucker ArthurZucker Nov 16, 2023

Choose a reason for hiding this comment

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

  • Vit
  • Clip
  • Detr (tests/models/detr/test_modeling_detr.py::DetrModelIntegrationTestsTimmBackbone::test_inference_panoptic_segmentation_head fails:
E       - {'id': 1, 'label_id': 17, 'score': 0.994097, 'was_fused': False}
E       ?                                           ^
E       
E       + {'id': 1, 'label_id': 17, 'score': 0.994096, 'was_fused': False}

Might be my arch rather than anything

  • SAM

Which models would you think rely a lot on pillow version that I should test as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that difference is definitely in the realm of acceptable!

All vision models rely on Pillow for their processing. I picked these models to cover different default resampling algos + popular models on different tasks. AFAICT - there's just bicubic, bilinear and nearest used for resizing settings and they've all been tested :)

@ArthurZucker ArthurZucker merged commit b54993a into main Nov 22, 2023
22 checks passed
@ArthurZucker ArthurZucker deleted the update-pillow branch November 22, 2023 08:40
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 this pull request may close these issues.

3 participants