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

feat: Improve read_database_uri typing #19334

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

max-muoto
Copy link
Contributor

@max-muoto max-muoto commented Oct 20, 2024

This PR improves the typing of read_database_uri by adding overloads so that the core set of exceptions it raises are caught by type-checkers, that being:

Trying to pass in excecute_options with the connector is connector-x:

# Raises, not caught by type-checking.
df = pl.read_database_uri(
    "SELECT * from user",
    "postgresql://localhost:5432/postgres",
    execute_options={"Test": None},
)

And, passing in a list of strings when adbc is the driver:

# Raises, not caught by type-checking.
df = pl.read_database_uri(
    ["SELECT 1", "SELECT * from user"],
    "postgresql://localhost:5432/postgres",
    engine="adbc",
)

I personally encountered the first, and would have valued from having this represented through the overloads.

@max-muoto max-muoto changed the title Improve read_database_uri overloads Improve read_database_uri typing Oct 20, 2024
@max-muoto max-muoto changed the title Improve read_database_uri typing feat: Improve read_database_uri typing Oct 20, 2024
@github-actions github-actions bot added enhancement New feature or an improvement of an existing feature python Related to Python Polars rust Related to Rust Polars and removed title needs formatting labels Oct 20, 2024
@max-muoto max-muoto marked this pull request as draft October 20, 2024 22:23


@overload
def read_database_uri(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When it's ambiguous which is being used, we only accept arguments that wouldn't error for either.

Copy link

codecov bot commented Oct 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.23%. Comparing base (08732c4) to head (c0a0295).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #19334      +/-   ##
==========================================
+ Coverage   80.21%   80.23%   +0.01%     
==========================================
  Files        1523     1523              
  Lines      209544   209544              
  Branches     2434     2434              
==========================================
+ Hits       168096   168123      +27     
+ Misses      40893    40866      -27     
  Partials      555      555              

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

@max-muoto max-muoto marked this pull request as ready for review October 21, 2024 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or an improvement of an existing feature python Related to Python Polars rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant