Skip to content

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

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

Conversation

Sachi-Parashar
Copy link

Summary

This PR addresses issue #849, where pyright fails to type-check the converters argument in read_excel when using functools.partial or lambda functions.

Fix

Updated all read_excel and parse overloads in pandas/io/excel/_base.pyi:

- converters: Mapping[int | str, Callable[[object], object]] | None
+ converters: Mapping[int | str, Callable[[Any], Any]] | None

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.

from functools import partial
import pandas as pd
from typing_extensions import assert_type

conv = partial(pd.to_datetime, errors="coerce")
df = pd.read_excel("foo.xlsx", converters={"col1": conv})
assert_type(df, pd.DataFrame)

Notes

  • Verified with both mypy and pyright. - Worked✅
  • If needed, I'm open to updating ConvertersArg in _typing.pyi for broader consistency in a follow-up PR.

Closes #849

@Sachi-Parashar Sachi-Parashar changed the title Add files via upload Jul 28, 2025
Comment on lines 1 to 9
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)
Copy link
Collaborator

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

Copy link
Author

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
Comment on lines +1798 to +1800
from functools import partial
import pandas as pd
from typing_extensions import assert_type
Copy link
Collaborator

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)
Copy link
Collaborator

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})
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants