Skip to content

Fix: modular image processors#45492

Merged
zucchini-nlp merged 1 commit intohuggingface:mainfrom
zucchini-nlp:fix-modular-image-processor
Apr 20, 2026
Merged

Fix: modular image processors#45492
zucchini-nlp merged 1 commit intohuggingface:mainfrom
zucchini-nlp:fix-modular-image-processor

Conversation

@zucchini-nlp
Copy link
Copy Markdown
Member

What does this PR do?

Discovered in #45075

This was added in the recent image processing refactor, but was not reverted after fixing pkg requirements. We should not be defining the same class with identical name two times in modular, defining it once should copy in both files and resolve deps

)


class LlavaOnevisionImageProcessorKwargs(ImagesKwargs, total=False):
Copy link
Copy Markdown
Member Author

@zucchini-nlp zucchini-nlp Apr 17, 2026

Choose a reason for hiding this comment

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

already defined in L58-59, delete duplicate

@github-actions
Copy link
Copy Markdown
Contributor

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

run-slow: llava_onevision

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

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.

@zucchini-nlp zucchini-nlp requested a review from vasqu April 17, 2026 13:58
Copy link
Copy Markdown
Contributor

@vasqu vasqu left a comment

Choose a reason for hiding this comment

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

SGTM

relative_dependency_order[dep],
# If this dependency is explicitly defined in the modular, prefer the modular's version.
# This prevents a renamed parent class from overriding a modular-defined class of the same name.
modular_mapper.global_nodes[dep] if dep in modular_mapper.classes else mapper.global_nodes[dep],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So this ternary was too gracious(?); makes sense to be more strict here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, wasn't needed imo

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No, this is needed!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

AT least it leads to deviations

@zucchini-nlp zucchini-nlp added this pull request to the merge queue Apr 20, 2026
Merged via the queue into huggingface:main with commit cba1e40 Apr 20, 2026
19 checks passed
@zucchini-nlp zucchini-nlp deleted the fix-modular-image-processor branch April 20, 2026 08:13
tarekziade added a commit that referenced this pull request Apr 20, 2026
github-merge-queue Bot pushed a commit that referenced this pull request Apr 20, 2026
lvliang-intel pushed a commit to lvliang-intel/transformers that referenced this pull request Apr 21, 2026
lvliang-intel pushed a commit to lvliang-intel/transformers that referenced this pull request Apr 21, 2026
artem-spector pushed a commit to artem-spector/transformers that referenced this pull request Apr 21, 2026
artem-spector pushed a commit to artem-spector/transformers that referenced this pull request Apr 21, 2026
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.

4 participants