Skip to content

Conversation

@MilkClouds
Copy link
Contributor

What does this PR do?

This PR fixes Auto classes (AutoImageProcessor and AutoVideoProcessor) to properly support dynamically registered processors via the .register() method.

Problem

When users call AutoImageProcessor.register() or AutoVideoProcessor.register(), the registration only updates the *_MAPPING objects (specifically their _extra_content), not the *_MAPPING_NAMES dictionaries. However, some code paths were checking *_MAPPING_NAMES instead of *_MAPPING, which meant registered processors would not be recognized in certain code paths.

Changes

image_processing_auto.py

  • Line 527-531: Replaced a for-else loop that checked if image_processor_type exists in IMAGE_PROCESSOR_MAPPING_NAMES.values() with a direct call to get_image_processor_class_from_name(), which properly checks both the static mapping and registered items via IMAGE_PROCESSOR_MAPPING._extra_content

video_processing_auto.py

  • Line 294: Changed from checking if video_processor_class_inferred in VIDEO_PROCESSOR_MAPPING_NAMES.values() to using video_processor_class_from_name(video_processor_class_inferred) is not None, which properly checks both the static mapping and registered items

Why This Matters

The helper functions like get_image_processor_class_from_name() and video_processor_class_from_name() are designed to check both:

  1. The static *_MAPPING_NAMES dictionaries (for built-in processors)
  2. The *_MAPPING._extra_content dictionaries (for dynamically registered processors)

By using these helper functions instead of directly checking *_MAPPING_NAMES.values(), we ensure that dynamically registered processors are properly recognized throughout the codebase.

Testing

The changes maintain backward compatibility with existing code while adding support for registered processors. The helper functions already have the logic to handle both static and dynamic mappings.

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?

@ArthurZucker @Cyrilvallez (model loading)

@github-actions
Copy link
Contributor

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

run-slow: auto

@MilkClouds
Copy link
Contributor Author

For reviewer's information: this PR is ready for review!

@Rocketknight1
Copy link
Member

This is a nice bug report, actually! I didn't think about how checking MAPPING_NAMES would miss custom registered classes. It's in core auto code though, so will wait for core maintainer review cc @ArthurZucker @Cyrilvallez

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.

2 participants