Skip to content

Fix: modular image processors - #45492

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

zucchini-nlp merged 1 commit into
huggingface: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):

@zucchini-nlp zucchini-nlp Apr 17, 2026 •

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.

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

@vasqu vasqu left a comment

Copy link
Copy Markdown
Collaborator

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
Collaborator

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
eustlb pushed a commit that referenced this pull request May 12, 2026
eustlb pushed a commit that referenced this pull request May 12, 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