-
-
Notifications
You must be signed in to change notification settings - Fork 145
Fix #849: Update converters type in read_excel for better Pyright compatibility #1297
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
base: main
Are you sure you want to change the base?
Fix #849: Update converters type in read_excel for better Pyright compatibility #1297
Conversation
tests/test_excel_converters.py
Outdated
from functools import partial | ||
import pandas as pd | ||
from typing_extensions import assert_type | ||
|
||
partial_func = partial(pd.to_datetime, errors="coerce") | ||
|
||
df = pd.read_excel("foo.xlsx", converters={"field_1": partial_func}) | ||
|
||
assert_type(df, pd.DataFrame) |
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.
This should go in test_io.py
and be in a test_converters_partial()
function
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.
Thanks for the suggestion @Dr-Irv — I’ve moved the test to test_io.py
as test_converters_partial()
and deleted the separate test file.
Remove redundant test_excel_converters.py after moving test to test_io.py
Add test_converters_partial() to test_io.py for read_excel converters
from functools import partial | ||
import pandas as pd | ||
from typing_extensions import assert_type |
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.
move the first import to top of file.
the second 2 imports are already there at the top, so remove from here.
|
||
partial_func = partial(pd.to_datetime, errors="coerce") | ||
df = pd.read_excel("foo.xlsx", converters={"field_1": partial_func}) | ||
assert_type(df, pd.DataFrame) |
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.
Use check(assert_type(df, pd.DataFrame), pd.DataFrame)
from typing_extensions import assert_type | ||
|
||
partial_func = partial(pd.to_datetime, errors="coerce") | ||
df = pd.read_excel("foo.xlsx", converters={"field_1": partial_func}) |
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.
This will need to work at runtime. If you look at test_read_excel()
, you will see the use of ensure_clean()
and that the test needs to first write out a file, and then read back the file. So you need to change to use that pattern.
Summary
This PR addresses issue #849, where
pyright
fails to type-check theconverters
argument inread_excel
when usingfunctools.partial
or lambda functions.Fix
Updated all
read_excel
andparse
overloads inpandas/io/excel/_base.pyi
:This relaxes the type signature to accept functools.partial and similar callables.
Test
Added a test tests/test_excel_converters.py with assert_type to confirm the fix works with pyright and mypy.
Notes
Closes #849