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

Define errors in errors.py #2170

Merged

Conversation

Y4suyuki
Copy link
Contributor

@Y4suyuki Y4suyuki commented Mar 29, 2024

Define errors in utils except utils/_errors.py in errors.py

#2069

@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.

@Y4suyuki
Copy link
Contributor Author

@Wauplin Hi, I made another PR for defining errors.
I put errors in utils/_errors.py aside for this PR since they seem a bit complex so I just defined simpler errors in this PR.
Also I thought importing errors in utils/__init__.py seems redundant after defined them in errors.py and tried to remove them but I noticed that will change API of this library so I didn't include those changes in this PR.

@Y4suyuki Y4suyuki marked this pull request as ready for review March 29, 2024 17:34
@Wauplin
Copy link
Contributor

Wauplin commented Mar 29, 2024

Thanks for the contribution and explanations @Y4suyuki! Your reasoning looks good. I'll have a look at your PR next week and let you know :) In the meantime, have a great weekend 🤗

@Y4suyuki Y4suyuki changed the title [WIP] Define errors in errors.py Define errors in errors.py Mar 30, 2024
Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

Thanks @Y4suyuki! I played with it a bit and it's definitely much cleaner now! 🎉

And thanks for taking care of not introducing breaking changes. I agree it makes imports redundant but for libraries like this one it's more important not to brake downstream usage rather than having a perfectly clean library internally.

@Wauplin Wauplin merged commit aaf23b4 into huggingface:main Apr 8, 2024
14 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.

None yet

3 participants