Simplify and standardize processor tests by yonigozlan · Pull Request #41773 · huggingface/transformers

yonigozlan · 2025-10-21T22:33:49Z

What does this PR do?

Improve ProcessorTestMixin to standardize processor tests, especially the setup part.
Requires #41633

…asses

…rom-processors

… (temporarily)

…rom-processors

…m/yonigozlan/transformers into remove-attributes-from-processors

HuggingFaceDocBuilderDev · 2025-10-22T18:04:38Z

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.

…sor-tests

…tests

zucchini-nlp

Nice clean-up. I didn't look at all the test files, if CI is green I'll assume they are fine :)

zucchini-nlp · 2025-11-10T09:22:10Z

            original_sizes = original_sizes.tolist()
-        if isinstance(reshaped_input_sizes, (torch.Tensor, np.ndarray)):
-            reshaped_input_sizes = reshaped_input_sizes.tolist()
+        # TODO: add connected components kernel for postprocessing


looks like still in progress? The args max_hole_area and max_sprinkle_area are not used by fn and I think modular didn't copy self._apply_non_overlapping_constraints

Yes I need to add the kernels for these in another PR, however reshaped_input_sizes is never needed in any case

Imo we need to do all changes in a different PR in that case, because the current changes look unfinished. Up to you though :)

I agree with @zucchini-nlp, let's keep the focus of this PR and do other changes in another PR all at once instead of adding TODOs etc!

zucchini-nlp · 2025-11-10T09:45:53Z

+        """
+        attributes = self.processor_class.get_attributes()
+
+        if not any(attr in ["tokenizer", "image_processor"] for attr in attributes):


we can support video_processor and feature_extractor, as those are another two common attributes

Added support for video_processor. However it looks like very few feature extractors actually use AudioKwargs, so it's difficult to set a kwargs that will work with all feature extractors safely. Maybe @eustlb you have more context to handle this?

yeah, i think we didn't pay much attention to audio models when updating. Might be smth on audio-team''s roadmap

zucchini-nlp · 2025-11-10T09:48:28Z

+        for key in input_image_proc:
+            self.assertAlmostEqual(input_image_proc[key].sum(), input_processor[key].sum(), delta=1e-2)


why do we need to sum instead of torch.assertallclose?

changed it thanks!

zucchini-nlp · 2025-11-10T09:55:17Z

+        # Process with both tokenizer and processor (disable padding to ensure same output)
+        try:
+            encoded_processor = processor(text=input_str, padding=False, return_tensors="pt")
+        except Exception:


should we catch a specific exception here (ValueError)?

I'm getting different type of error there, so hard to specify

…tests

yonigozlan · 2025-11-11T19:18:32Z

@Cyrilvallez This should be ready to merge!
@ydshieh The failing tests here are also failing on main:
test_overflowing_tokens is failing for test_processing_udop, test_processing_layoutlmv2 and test_processing_layoutxlm. It looks like a missing repo on the hub?

ydshieh · 2025-11-12T11:03:49Z

Hi @yonigozlan

For LayoutLMv2ProcessorTest, after the merged PR

[v5] 🚨Refactor subprocessors handling in processors (#41633)
the error is

commit: f065e40

FAILED tests/models/layoutlmv2/test_processing_layoutlmv2.py::LayoutLMv2ProcessorTest::test_overflowing_tokens - OSError: Unable to load vocabulary from file. Please check that the provided vocabulary is accessible and not corrupted.

Prior, it was

commit: 91d250e

FAILED tests/models/layoutlmv2/test_processing_layoutlmv2.py::LayoutLMv2ProcessorTest::test_overflowing_tokens - ValueError: Words must be of type `List[str]` (single pretokenized example), or `List[List[str]]` (batch of pretokenized examples).

Could you check this?

I will check what I can do once the error being back to the same error. They are failing for quite some time 😢

Cyrilvallez

Nice cleanup! Left some comments but as I'm not super familiar with processor tests, feel free to disregard if out-of-scope. If @zucchini-nlp is onboard, then feel free to merge!

Cyrilvallez · 2025-11-17T11:47:06Z

            original_sizes = original_sizes.tolist()
-        if isinstance(reshaped_input_sizes, (torch.Tensor, np.ndarray)):
-            reshaped_input_sizes = reshaped_input_sizes.tolist()
+        # TODO: add connected components kernel for postprocessing


I agree with @zucchini-nlp, let's keep the focus of this PR and do other changes in another PR all at once instead of adding TODOs etc!

Cyrilvallez · 2025-11-17T11:54:28Z

-        components = self.prepare_components()
-        processor = self.processor_class(**components, **self.prepare_processor_dict())
+        processor = self.processor_class.from_pretrained(self.tmpdirname)


Isn't that assuming that saving/loading works perfectly? I.e. is it not clashing a bit with the test philosophy as we also test that saving and loading are behaving as they shiould

Yes i see what you mean. We have several tests that test if we get the same processor after saving and reloading with save and from_pretrained. However we don't have a test to check that loading a processor with from pretrained is equivalent to loading each sub-components separately with from pretrained and instantiating the processor with these loaded sub components after, so I just added one (test_processor_from_pretrained_vs_from_components).

Cyrilvallez · 2025-11-17T11:55:42Z

+    @classmethod
+    def _setup_test_attributes(cls, processor):
+        # to override in the child class to define class attributes
+        # such as image_token, video_token, audio_token, etc.
+        pass


Would maybe be a bit clearer/easier to do in __init__?

I prefer to have this as a separate method, as it will be called by the setUpClass which can pass it a processor object. It might be weird to have init not be the entrypoint of the object

…tests

yonigozlan · 2025-11-24T23:29:07Z

@zucchini-nlp @Cyrilvallez Thanks for the reviews! Waiting on your approval to merge now :)

One thing to note, I had to add a config for layoutxlm to have all tests pass. This hides a deeper issues, basically we have a few models in the library that don't have a config (namely code_llama, gpt-sw3, granitevision and nougat), but this messes up completely the "SUBPROCESSOR"_MAPPING that are built as _LazyAutoMapping objects in auto_factory.py. _LazyAutoMapping defines a _reverse_config_mapping dict, but if a config is used for several models in CONFIG_MAPPING_NAMES in configuration_auto.py, all but one of the mapping will be overwritten.

Imo the solution would be to always have a config when adding a model, especially now with modular, no reason not to have that.

I'll open a PR to enforce that, unless you think there is another way to handle the issue.

…tests

zucchini-nlp

The Layout config addition looks fine to me. I wonder how we ended up using the same config with two models given the "one model - one file" philosophy. If get special model arch which can't map correctly with config mapping, a custom setup_{attribute} is another way imo

Haven't looked at all model tests, the common tests look good though. Thanks!

zucchini-nlp · 2025-11-26T10:36:38Z

-        rope_deltas (`torch.LongTensor` of shape `(batch_size, )`, *optional*):
-            The rope index difference between sequence length and multimodal rope.


Oh thanks, the repo check complains on the docs order. Just noticed that the arg isn't even in the signature 😆

zucchini-nlp · 2025-11-26T10:38:05Z

        vocab_size (`int`, *optional*, defaults to 30522):
-            Vocabulary size of the LayoutLMv2 model. Defines the number of different tokens that can be represented by
-            the `inputs_ids` passed when calling [`LayoutLMv2Model`] or [`TFLayoutLMv2Model`].
+            Vocabulary size of the LayoutXLM model. Defines the number of different tokens that can be represented by


These changes arent' needed, no? The config is called LayoutLMv2Config

Ah yes my bad, thanks for catching that

zucchini-nlp · 2025-11-26T10:39:26Z

+    Args:
+        vocab_size (`int`, *optional*, defaults to 30522):
+            Vocabulary size of the LayoutXLM model. Defines the number of different tokens that can be represented by
+            the `inputs_ids` passed when calling [`LayoutXLMModel`] or [`TFLayoutXLMModel`].


not super related to the PR, looks like there are a few references to a non-existing TF class in the docs

zucchini-nlp · 2025-11-26T11:17:34Z

+        has_image_processor = "image_processor" in attributes
+        if has_image_processor:
+            additional_kwargs["do_normalize"] = False
+        has_video_processor = "video_processor" in attributes
+        if has_video_processor:
+            additional_kwargs["do_normalize"] = False


this is an interesting point I have been thinking recently, especially with the use_fast flag. We have no way for users to override kwargs from one attribute only. The kwargs are passed to all processor's attributes

Maybe we can consider after v5 making it separate?

Yes indeed that could be useful. I guess users can still load the subcomponents separately with from_pretrained and different use_fast flags, but not ideal indeed.

yeah, I just remembered one person was trying to load a processor with slow image processor and fast tokenizer, because the model has no support for a slow tokenizer

github-actions · 2025-11-26T17:33:44Z

[For maintainers] Suggested jobs to run (before merge)

run-slow: auto, edgetam, gemma3n, glm46v, glm4v, glm4v_moe, layoutlmv2, layoutxlm, llava_next, llava_onevision, mgp_str, owlvit, sam, sam2, sam2_video, sam3

* remove attributes and add all missing sub processors to their auto classes * remove all mentions of .attributes * cleanup * fix processor tests * fix modular * remove last attributes * fixup * fixes after merge * fix wrong tokenizer in auto florence2 * fix missing audio_processor + nits * Override __init__ in NewProcessor and change hf-internal-testing-repo (temporarily) * fix auto tokenizer test * add init to markup_lm * update CustomProcessor in custom_processing * remove print * nit * refactor processor tests first part * refactor part 2 * fix test modeling owlv2 * fix test_processing_layoutxlm * Fix owlv2, wav2vec2, markuplm, voxtral issues * part3 * refactor all processor with mixin * add support for loading and saving multiple tokenizer natively * remove exclude_attributes from save_pretrained * get processor from pretrained instead of components in tests * skip tests in colqwen2, pixtral * modifs after review * fix style and copies * Fix after review * add test_processor_from_pretrained_vs_from_components, fix failing tests * fix overflowing_tokens tests * add config for layoutxlm * fix ci * use modular * fic docstring * Standardize mgp_str tests * fix after review

yonigozlan and others added 23 commits October 15, 2025 15:47

remove attributes and add all missing sub processors to their auto cl…

f48a47b

…asses

remove all mentions of .attributes

d5d5c58

cleanup

dd505b5

fix processor tests

6a1448f

fix modular

a292900

remove last attributes

63a255d

fixup

ef73759

Merge remote-tracking branch 'upstream/main' into remove-attributes-f…

b5e8b2e

…rom-processors

fixes after merge

f14ff3c

fix wrong tokenizer in auto florence2

0306430

fix missing audio_processor + nits

01cb815

Override __init__ in NewProcessor and change hf-internal-testing-repo…

49ec906

… (temporarily)

Merge remote-tracking branch 'upstream/main' into remove-attributes-f…

7dd5682

…rom-processors

fix auto tokenizer test

946cc5c

add init to markup_lm

b0cb3e0

update CustomProcessor in custom_processing

3b9e846

remove print

53de7a4

Merge branch 'main' into remove-attributes-from-processors

93d2c4d

Merge remote-tracking branch 'upstream/main' into remove-attributes-f…

feeec28

…rom-processors

nit

4a6b080

Merge branch 'remove-attributes-from-processors' of https://github.co…

02402a0

…m/yonigozlan/transformers into remove-attributes-from-processors

refactor processor tests first part

9204b4c

refactor part 2

1ed7c56

yonigozlan force-pushed the simplify-processor-tests branch from 8e12561 to 1ed7c56 Compare October 22, 2025 17:55

yonigozlan added 5 commits October 22, 2025 18:19

fix test modeling owlv2

757e1f1

fix test_processing_layoutxlm

bf763b2

Fix owlv2, wav2vec2, markuplm, voxtral issues

0799a0a

part3

98ead2c

refactor all processor with mixin

59234ee

yonigozlan added 3 commits November 7, 2025 18:39

Merge branch 'remove-attributes-from-processors' into simplify-proces…

447b598

…sor-tests

Merge remote-tracking branch 'upstream/main' into simplify-processor-…

ac72ba2

…tests

fix style and copies

d5bf14a

yonigozlan requested review from Cyrilvallez and zucchini-nlp November 7, 2025 19:00

zucchini-nlp reviewed Nov 10, 2025

View reviewed changes

yonigozlan added 2 commits November 11, 2025 19:04

Fix after review

773342b

Merge remote-tracking branch 'upstream/main' into simplify-processor-…

12c854c

…tests

Cyrilvallez reviewed Nov 17, 2025

View reviewed changes

yonigozlan added 6 commits November 24, 2025 17:59

Merge remote-tracking branch 'upstream/main' into simplify-processor-…

12a01fd

…tests

add test_processor_from_pretrained_vs_from_components, fix failing tests

7d7c6b2

fix overflowing_tokens tests

fa94bcb

add config for layoutxlm

74492e5

fix ci

9bd9da1

use modular

e4e36d9

fic docstring

1fd0cd5

yonigozlan added the for_v5? label Nov 25, 2025

yonigozlan added 2 commits November 25, 2025 22:32

Standardize mgp_str tests

1c21d90

Merge remote-tracking branch 'upstream/main' into simplify-processor-…

d931a2b

…tests

yonigozlan requested review from Cyrilvallez and zucchini-nlp November 25, 2025 23:34

zucchini-nlp approved these changes Nov 26, 2025

View reviewed changes

fix after review

572b26d

yonigozlan enabled auto-merge (squash) November 26, 2025 17:40

yonigozlan merged commit 06d52fe into huggingface:main Nov 26, 2025
23 checks passed

		for key in input_image_proc:
		self.assertAlmostEqual(input_image_proc[key].sum(), input_processor[key].sum(), delta=1e-2)

		rope_deltas (`torch.LongTensor` of shape `(batch_size, )`, optional):
		The rope index difference between sequence length and multimodal rope.

Conversation

yonigozlan commented Oct 21, 2025

What does this PR do?

Uh oh!

HuggingFaceDocBuilderDev commented Oct 22, 2025

Uh oh!

zucchini-nlp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yonigozlan commented Nov 11, 2025

Uh oh!

ydshieh commented Nov 12, 2025

Uh oh!

Cyrilvallez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yonigozlan commented Nov 24, 2025

Uh oh!

zucchini-nlp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects