Skip to content

Conversation

@topper-123
Copy link
Contributor

Adds return types to read_sql|read_sql_query|read_sql_table.

I'd like to take on the other pd.read_* functions in follow-ups.

Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

looks pretty good. I think the second overload in each case is unnecessary though

@simonjayhawkins

pandas/io/sql.py Outdated
...


@overload
Copy link
Member

Choose a reason for hiding this comment

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

Hmm I don't think an overload should match the overloaded-signature should it? I think can delete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's right. I've updated the PR to remove all unneeded overloads.

@WillAyd WillAyd added the Typing type annotations, mypy/pyright type checking label May 18, 2020
@topper-123 topper-123 force-pushed the typ_sql_functions branch from 790129a to 16d478d Compare May 19, 2020 16:15
params=None,
parse_dates=None,
chunksize: None = None,
) -> Iterator[DataFrame]:
Copy link
Member

Choose a reason for hiding this comment

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

are the return types of the two overloads switched? or have a misread the docstring?

Copy link
Member

Choose a reason for hiding this comment

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

Yea looks like a copy / paste error? The other two funcs look correct.

lgtm outside of this issue

@simonjayhawkins simonjayhawkins added this to the 1.1 milestone May 20, 2020
@topper-123
Copy link
Contributor Author

Yeah, that was a mistake. I've updated.

@simonjayhawkins simonjayhawkins merged commit c5ea16f into pandas-dev:master May 21, 2020
@simonjayhawkins
Copy link
Member

Thanks @topper-123

@topper-123 topper-123 deleted the typ_sql_functions branch May 21, 2020 09:58
@simonjayhawkins simonjayhawkins mentioned this pull request Jun 2, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Typing type annotations, mypy/pyright type checking

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants