Skip to content

Add some missing Optional type annotations to interval_range and read_sql #1315

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 12, 2025
Merged

Add some missing Optional type annotations to interval_range and read_sql #1315

merged 1 commit into from
Aug 12, 2025

Conversation

MarcoGorelli
Copy link
Member

These are some cases where the overloads indicate that they are meant to match the parameter defaults, which in all these cases are = None. However, None is currently missing from the annotations for these parameters

  • Closes #xxxx (Replace xxxx with the Github issue number)
  • Tests added: Please use assert_type() to assert the type of any return value

MarcoGorelli
MarcoGorelli commented Aug 11, 2025
View reviewed changes
MarcoGorelli
MarcoGorelli commented Aug 11, 2025
View reviewed changes
@MarcoGorelli MarcoGorelli marked this pull request as draft August 11, 2025 17:27
@MarcoGorelli MarcoGorelli changed the title add some missing Optional type annotations WIP add some missing Optional type annotations Aug 11, 2025
Dr-Irv
Dr-Irv requested changes Aug 11, 2025
View reviewed changes
Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

If you want to fix the other defaults for read_sql() that would be good as well, but not a problem if you don't.

@MarcoGorelli MarcoGorelli changed the title WIP add some missing Optional type annotations Add some missing Optional type annotations Aug 12, 2025
@MarcoGorelli MarcoGorelli changed the title Add some missing Optional type annotations Add some missing Optional type annotations to interval_range and read_sql Aug 12, 2025
MarcoGorelli
MarcoGorelli commented Aug 12, 2025
View reviewed changes
@MarcoGorelli MarcoGorelli marked this pull request as ready for review August 12, 2025 08:16
@MarcoGorelli
Copy link
Member Author

Thanks @Dr-Irv for your review!

I noticed these missing | Nones while looking into a follow-up to #1293. There's quite a few more that are missing. For ease of review, OK if we start with fixing the missing | Nones, and in a follow-up I'll address the defaults?

@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Aug 12, 2025

I noticed these missing | Nones while looking into a follow-up to #1293. There's quite a few more that are missing. For ease of review, OK if we start with fixing the missing | Nones, and in a follow-up I'll address the defaults?

Yes, that's fine.

@Dr-Irv Dr-Irv merged commit 2228223 into pandas-dev:main Aug 12, 2025
13 checks passed
Dr-Irv
Dr-Irv reviewed Aug 12, 2025
View reviewed changes
Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

Thanks @MarcoGorelli

@MarcoGorelli MarcoGorelli mentioned this pull request Aug 12, 2025
Merged
2 tasks
@Dr-Irv Dr-Irv mentioned this pull request Aug 12, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dr-Irv Dr-Irv Dr-Irv requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MarcoGorelli @Dr-Irv