Skip to content

Conversation

namgyu-youn
Copy link

What does this PR do?

There was no test module for ViTImageProcessorFast although it was already implemented. For resolving that issue, this PR builds test module for ViTImageProcessorFast

Related Issue/PR: #36978

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

cc @amyeroberts, @qubvel

@namgyu-youn
Copy link
Author

Test Result:

$ python -m unittest tests/utils/test_image_processing_utils.py
.sssssssss.Using a slow image processor as `use_fast` is unset and a slow processor was saved with this model. `use_fast=True` will be the default behavior in v4.52, even if the model was saved with a slow processor. This will result in minor differences in outputs. You'll still be able to use a slow processor with `use_fast=False`.
/home/elicer/.local/lib/python3.10/site-packages/transformers/models/clip/feature_extraction_clip.py:30: FutureWarning: The class CLIPFeatureExtractor is deprecated and will be removed in version 5 of Transformers. Please use CLIPImageProcessor instead.
  warnings.warn(
..
----------------------------------------------------------------------
Ran 13 tests in 1.549s

OK (skipped=9)

@qubvel
Copy link
Member

qubvel commented Aug 12, 2025

cc @yonigozlan

Copy link
Member

@yonigozlan yonigozlan left a comment

Choose a reason for hiding this comment

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

Hi @namgyu-youn ! Thanks for contributing, ok to merge this for me after removing the redundant test, but let's change the name of the PR, as this adds general image processor tests, taking ViT image processors as an example.
There are already unit tests for Vit image processors in /tests/models/vit/test_image_processing_vit.py, so the title is a bit misleading

Comment on lines 68 to 76
def test_vit_processors_compatibility(self):
processor = ViTImageProcessor.from_pretrained("hf-internal-testing/tiny-random-vit")
processor_fast = ViTImageProcessorFast.from_pretrained("hf-internal-testing/tiny-random-vit")

# Check that both processors have similar configurations
self.assertEqual(processor.size, processor_fast.size)
self.assertEqual(processor.do_resize, processor_fast.do_resize)
self.assertEqual(processor.do_rescale, processor_fast.do_rescale)
self.assertEqual(processor.do_normalize, processor_fast.do_normalize)
Copy link
Member

Choose a reason for hiding this comment

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

we have extensive tests for this already, no need to add here

@namgyu-youn namgyu-youn changed the title build unittest for ViTImageProcessorFast add test case for ViTImageProcessorFast Aug 12, 2025
@namgyu-youn namgyu-youn changed the title add test case for ViTImageProcessorFast add fast image processor test case for ViT Aug 12, 2025
@namgyu-youn namgyu-youn requested a review from yonigozlan August 12, 2025 23:29
@namgyu-youn
Copy link
Author

namgyu-youn commented Aug 13, 2025

@yonigozlan Please feel free to rename title if there is a better one.

@yonigozlan yonigozlan changed the title add fast image processor test case for ViT add general hub test for Fast Image Processors in test_image_processing_utils Aug 14, 2025
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Member

@Cyrilvallez Cyrilvallez left a comment

Choose a reason for hiding this comment

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

Alright @yonigozlan, if you think the tests are useful feel free to merge it! Thanks @namgyu-youn! 🤗

@namgyu-youn
Copy link
Author

namgyu-youn commented Aug 16, 2025

@yonigozlan Hi, I have a question before merging this PR. As we know, there are many image processor (‎tests/models/<MODEL_NAME>/test_image_processing_<MODEL_NAME>.py) unittest. And it seems wrong skipTest is being used.

SkipTest happens if import torch or import <MODEL_NAME>ImageProcessor is unavailable. But skipping these is not a good convention because we have to import them for running test. That is, the test should return fail/error when we are unavailable to import libraries. Therefore, I want to suggest removing SkipTest across the test module (with more brevity).

Actually, I already opened a PR (#40124) for this. Could you check #40124 ? If it is, I want to apply it across the image processor (‎tests/models/<MODEL_NAME>/test_image_processing_<MODEL_NAME>.py) unittest. This refactoring will make it more brevity.

@namgyu-youn
Copy link
Author

@ArthurZucker @Cyrilvallez @yonigozlan Could you please merge this PR? It hasn't merged yet after 3 approval.

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.

6 participants