-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Move exceptions.py
to utils/exceptions.py
#6296
Conversation
exceptions
to utils/exceptions.py
exceptions.py
to utils/exceptions.py
Show benchmarksPyArrow==8.0.0 Show updated benchmarks!Benchmark: benchmark_array_xd.json
Benchmark: benchmark_getitem_100B.json
Benchmark: benchmark_indices_mapping.json
Benchmark: benchmark_iterating.json
Benchmark: benchmark_map_filter.json
Show updated benchmarks!Benchmark: benchmark_array_xd.json
Benchmark: benchmark_getitem_100B.json
Benchmark: benchmark_indices_mapping.json
Benchmark: benchmark_iterating.json
Benchmark: benchmark_map_filter.json
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put the exceptions
module at the root of the project in purpose because it is part of the public API.
Note that having the exceptions/errors module at the root is a common pattern followed in many open-source libraries, like numpy
, pandas
, pyarrow
, requests
...
I'd rather be consistent with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could ask huggingface_hub
to align with the rest of open-source libraries and expose the errors/exceptions at the root of the library...
In [11]: requests.ConnectionError
Out[11]: requests.exceptions.ConnectionError
In [12]: pandas.errors.ClosedFileError
Out[12]: pandas.errors.ClosedFileError
In [13]: numpy.AxisError # defined in numpy/exceptions.py
Out[13]: numpy.AxisError
In [14]: pyarrow.ArrowKeyError # defined in pyarrow/error.pxi
Out[14]: pyarrow.lib.ArrowKeyError
Ok, I'll close this PR.
cc @Wauplin It would be nice to have an HF style guide to ensure consistency across our libraries 🙂. |
I can expose exceptions at root level yes. About having guidelines and consistency, let's try to do our best but it's not really in the essence of HF to formalize stuff in libraries 😒 |
Better late than never, we now have all the exceptions defined in |
Thanks for taking care, @Wauplin. I am closing this PR. |
I didn't notice the path while reviewing the PR yesterday :(