Skip to content

Conversation

@charchit7
Copy link
Contributor

What does this PR do?

Fixes #9567
Refactors image_processor file!

Before submitting

Who can review?

@a-r-r-o-w @stevhliu @yiyixuxu

@charchit7 charchit7 changed the title refactor image_processor file refactor image_processor.py file Oct 8, 2024
@charchit7 charchit7 force-pushed the refactor_image_processor branch from 4984d48 to 5d98fcc Compare October 9, 2024 06:39
Copy link
Contributor

@a-r-r-o-w a-r-r-o-w left a comment

Choose a reason for hiding this comment

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

Sweet, looks great!

@a-r-r-o-w a-r-r-o-w requested a review from stevhliu October 9, 2024 12:07
@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

@stevhliu stevhliu left a comment

Choose a reason for hiding this comment

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

Very nice, thank you! 🤗

@charchit7 charchit7 requested a review from a-r-r-o-w October 10, 2024 14:59
Copy link
Member

@stevhliu stevhliu left a comment

Choose a reason for hiding this comment

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

One last comment, then we can merge! 🤗

@charchit7 charchit7 requested a review from stevhliu October 10, 2024 20:11
@a-r-r-o-w
Copy link
Contributor

Could you run make style? Okay to merge after quality tests pass since we have stevehliu's approval

@charchit7
Copy link
Contributor Author

Done @a-r-r-o-w :)

@a-r-r-o-w
Copy link
Contributor

This puts it back to where it was when stevehliu asked for the last update. I think you need to fix the indent and put new width/height at same indent level as other arguments. Currently ruff thinks these are part of the previous argument due to wrong indent

@charchit7
Copy link
Contributor Author

This puts it back to where it was when stevehliu asked for the last update

Yeah, just noticed. Sorry, let me check.

@charchit7
Copy link
Contributor Author

Hey @a-r-r-o-w , let me know if it's okay now.

@charchit7 charchit7 requested a review from a-r-r-o-w October 11, 2024 11:09
@a-r-r-o-w
Copy link
Contributor

Looks good to merge! We're facing some CI environment issues causing all docs related things to fail, so will need to wait until it's back up

@a-r-r-o-w
Copy link
Contributor

a-r-r-o-w commented Oct 12, 2024

@sayakpaul Now seeing an unrelated error (apart from the docs environment ones) here. Seems like we need to bump transformers version, or is it something else?

Either way, I think this PR should be good to merge now as failing tests are unrelated, and I don't see any docs errors locally.

@a-r-r-o-w a-r-r-o-w merged commit 92d2baf into huggingface:main Oct 15, 2024
15 checks passed
sayakpaul pushed a commit that referenced this pull request Dec 23, 2024
* refactor image_processor file

* changes as requested

* +1 edits

* quality fix

* indent issue

---------

Co-authored-by: Aryan <[email protected]>
Co-authored-by: YiYi Xu <[email protected]>
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.

[community] Improving docstrings and type hints

5 participants