Skip to content

Conversation

@hlky
Copy link
Contributor

@hlky hlky commented Mar 6, 2025

What does this PR do?

Uses encoding="utf-8" and newline="\n"

FYI huggingface_hub introduced a style bot GitHub action huggingface/huggingface_hub#2898 we're using that in Diffusers https://github.com/huggingface/diffusers/blob/main/.github/workflows/pr_style_bot.yml to reduce the burden on external contributors.

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@github-actions github-actions bot marked this pull request as draft March 6, 2025 11:56
@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2025

Hi 👋, thank you for opening this pull request! The pull request is converted to draft by default. When it is ready for review, please click the Ready for review button (at the bottom of the PR page).

@hlky hlky marked this pull request as ready for review March 6, 2025 11:57
@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.

@Rocketknight1
Copy link
Member

Hi @hlky, this looks good! I think the newline kwarg is only needed on the writer file handle (mode="w"), though. On the reader file handles, universal newlines are already used, so setting newline="\n" will have no effect, and might even reduce compatibility (it will break if the input file has "\r\n" newlines): https://docs.python.org/3/library/functions.html#open

@hlky
Copy link
Contributor Author

hlky commented Mar 6, 2025

Thanks @Rocketknight1. .gitattributes will handle CRLF->LF if newline="\n" wasn't set on write but avoids the warning.

warning: in the working copy of 'src/transformers/models/siglip2/modeling_siglip2.py', CRLF will be replaced by LF the next time Git touches it

Copy link
Member

@Rocketknight1 Rocketknight1 left a comment

Choose a reason for hiding this comment

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

Yep, looks good to me now!

@Rocketknight1 Rocketknight1 merged commit bc30dd1 into huggingface:main Mar 6, 2025
10 checks passed
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.

3 participants