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

depr(python): Remove re-export of exceptions at top-level #17059

Merged
merged 3 commits into from
Jun 19, 2024

Conversation

stinodego
Copy link
Member

@stinodego stinodego commented Jun 19, 2024

Removing the top-level re-exports has some minor benefits:

  • Unify the way people handle exceptions. Right now we re-export both the exceptions module and the exceptions themselves. We should pick one.
  • Reduce clutter in the top-level namespace. This improves IDE hints and is especially relevant as we expand the exception hierarchy.
  • Easier to maintain. There are a lot of exceptions - we were already pretty bad at consistently re-exporting all of them, which led to only some exceptions being available top-level, the others only in the exceptions module (I straightened this out recently). This is a bad user experience.

Drawbacks:

  • If you have to handle errors, having them available under e.g. pl.ComputeError is quite convenient (though pl.exceptions.ComputeError isn't that much harder).
  • There are probably quite a few people using except pl.ComputeError, and this (loudly) breaks their code they will get a deprecation message.

For reference, pandas and NumPy also have a dedicated module for their exceptions and do not re-export at the top-level (NumPy only does for backward compatibility).

Example

Before:

try:
    ...
except pl.ComputeError:
    ...

Use instead:

from polars.exceptions import ComputeError

try:
    ...
except ComputeError:
    ...

@github-actions github-actions bot added breaking Change that breaks backwards compatibility enhancement New feature or an improvement of an existing feature python Related to Python Polars labels Jun 19, 2024
Copy link

codecov bot commented Jun 19, 2024

Codecov Report

Attention: Patch coverage is 73.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 80.86%. Comparing base (4d15fd4) to head (c8a1b64).
Report is 4 commits behind head on main.

Files Patch % Lines
py-polars/polars/dependencies.py 42.85% 1 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #17059      +/-   ##
==========================================
- Coverage   80.89%   80.86%   -0.03%     
==========================================
  Files        1455     1455              
  Lines      190961   191027      +66     
  Branches     2723     2724       +1     
==========================================
  Hits       154473   154473              
- Misses      35984    36050      +66     
  Partials      504      504              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stinodego stinodego marked this pull request as draft June 19, 2024 09:43
@stinodego stinodego changed the title feat(python)!: Remove re-export of exceptions at top-level depr(python): Remove re-export of exceptions at top-level Jun 19, 2024
@stinodego stinodego removed the breaking Change that breaks backwards compatibility label Jun 19, 2024
@github-actions github-actions bot added the deprecation Add a deprecation warning to outdated functionality label Jun 19, 2024
@stinodego stinodego marked this pull request as ready for review June 19, 2024 12:11
@stinodego stinodego merged commit 760067c into main Jun 19, 2024
16 checks passed
@stinodego stinodego deleted the remove-exception-reexport branch June 19, 2024 14:42
Wouittone pushed a commit to Wouittone/polars that referenced this pull request Jun 22, 2024
@c-peters c-peters added the accepted Ready for implementation label Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation deprecation Add a deprecation warning to outdated functionality enhancement New feature or an improvement of an existing feature python Related to Python Polars
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants