Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: typing issues, and replace deprecated python typing (Optional, Union) to | #1363

Merged
merged 1 commit into from
Apr 27, 2023

Conversation

kiyoon
Copy link
Contributor

@kiyoon kiyoon commented Apr 27, 2023

Fix typing issue in #1359 partially. For now, it only changes one file, but we'll need to gradually change all files to follow the new format.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Apr 27, 2023

The documentation is not available anymore as the PR was closed or merged.

@kiyoon
Copy link
Contributor Author

kiyoon commented Apr 27, 2023

I can undo the documentation changes if that's more desired

@kiyoon kiyoon marked this pull request as draft April 27, 2023 13:23
Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Nice, thanks a lot! @muellerzr could you also have a look?

Copy link
Collaborator

@muellerzr muellerzr left a comment

Choose a reason for hiding this comment

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

This all looks good to me! Definitely smaller PR's modifying a file or two at a time would be more feasible for us to review if you would like to. Thanks for updating it all, looks much cleaner! (So, what you have just here works great)

@kiyoon
Copy link
Contributor Author

kiyoon commented Apr 27, 2023

Thanks for reviewing! So, I should still remove the changes in the doc right?

@sgugger
Copy link
Collaborator

sgugger commented Apr 27, 2023

No those changes are good. It's an oversight on our side. You can just mark your PR as ready and we'll merge.

@kiyoon kiyoon marked this pull request as ready for review April 27, 2023 14:19
@muellerzr muellerzr merged commit 2708c1a into huggingface:main Apr 27, 2023
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