Fix pd.read_excel #33

hauntsaninja · 2022-06-21T04:55:51Z

No description provided.

Dr-Irv · 2022-06-21T12:33:53Z

@hauntsaninja Can you add a test for this? See test_frame.py:test_read_csv() .

hauntsaninja · 2022-07-01T05:21:39Z

Sorry, was away from laptop. I can add a test, but rather than effectively recreating a large part of pandas test suite, you may wish to consider using a tool like stubtest for basic consistency checks (#11 (comment))

Dr-Irv · 2022-07-01T11:51:13Z

tests/test_frame.py

@@ -1012,6 +1016,12 @@ def test_frame_getitem_isin() -> None:
    assert_type(df[df.index.isin([1, 3, 5])], "pd.DataFrame")


+def test_read_excel() -> None:


can you add a pytest.skip() here, as this will fail with pytest, since foo doesn't exist.

Also add a comment referring to the Github issue, e.g. # GH 33

Dr-Irv · 2022-07-01T11:52:52Z

I've run stubtest and it uncovered a number of issues, so I'm going to look into addressing those, and adding stubtest to our testing suite.

hauntsaninja · 2022-07-01T18:38:41Z

Thanks, fixed! stubtest has the option of using an allowlist, which can let you introduce it in CI today, and fix the backlog of errors over time. Let me know if you need help setting it up!

Dr-Irv · 2022-07-01T18:45:17Z

Thanks, fixed! stubtest has the option of using an allowlist, which can let you introduce it in CI today, and fix the backlog of errors over time. Let me know if you need help setting it up!

Thanks for letting me know and your offer of help. The challenge is that we don't know what to put on the allowlist.

I'm hoping to fix all the issues this weekend.

Dr-Irv · 2022-07-01T18:55:37Z

Thanks @hauntsaninja

Dr-Irv · 2022-07-03T18:25:44Z

@hauntsaninja I tried stubtest, and can't get it to do what I'd like it to do. mypy requires stub files to be installed, but I want to use this in development. So in development, I have a local copy of pandas-stubs, and want to run stubtest so that it says "here is what is declared/imported in the installed version of pandas, but is missing from pandas-stubs"

If I install pandas-stubs, then I get complaints about third-party libraries that I don't care about.

Any suggestions on how to handle this use case would be welcome.

hauntsaninja · 2022-07-06T10:48:37Z

Sorry for the slow reply, I'm on vacation, so only checking my laptop intermittently.

@Dr-Irv stubtest requires mypy to run correctly on the stubs before it actually performs its checks. This is what it means when it says "error: not checking stubs due to mypy build errors"

λ python -m mypy.stubtest pandas --concise
error: not checking stubs due to mypy build errors:
pandas-stubs/plotting/_matplotlib/converter.pyi:1: error: Skipping analyzing "matplotlib.dates": module is installed, but missing library stubs or py.typed marker
pandas-stubs/plotting/_matplotlib/converter.pyi:1: error: Skipping analyzing "matplotlib": module is installed, but missing library stubs or py.typed marker
pandas-stubs/plotting/_matplotlib/converter.pyi:2: error: Skipping analyzing "matplotlib.ticker": module is installed, but missing library stubs or py.typed marker
pandas-stubs/plotting/_matplotlib/converter.pyi:6: error: Skipping analyzing "matplotlib.units": module is installed, but missing library stubs or py.typed marker
pandas-stubs/plotting/_matplotlib/boxplot.pyi:3: error: Skipping analyzing "matplotlib.axes": module is installed, but missing library stubs or py.typed marker
pandas-stubs/plotting/_misc.pyi:11: error: Skipping analyzing "matplotlib.axes": module is installed, but missing library stubs or py.typed marker
pandas-stubs/plotting/_misc.pyi:12: error: Skipping analyzing "matplotlib.figure": module is installed, but missing library stubs or py.typed marker
pandas-stubs/plotting/_core.pyi:8: error: Skipping analyzing "matplotlib.axes": module is installed, but missing library stubs or py.typed marker
pandas-stubs/core/series.pyi:23: error: Skipping analyzing "matplotlib.axes": module is installed, but missing library stubs or py.typed marker
pandas-stubs/core/groupby/generic.pyi:18: error: Skipping analyzing "matplotlib.axes": module is installed, but missing library stubs or py.typed marker
pandas-stubs/core/frame.pyi:24: error: Skipping analyzing "matplotlib.axes": module is installed, but missing library stubs or py.typed marker
pandas-stubs/core/frame.pyi:24: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
pandas-stubs/_libs/tslibs/timestamps.pyi:157: error: Return type "Tuple[int, int, int]" of "isocalendar" incompatible with return type "_IsoCalendarDate" in supertype "datetime"
pandas-stubs/_libs/tslibs/timestamps.pyi:157: error: Return type "Tuple[int, int, int]" of "isocalendar" incompatible with return type "_IsoCalendarDate" in supertype "date"

So it looks to me like there are two issues with the stubs that need to be fixed before stubtest can actually run.

First, mypy needs to be able to resolve the reference to matplotlib. This is easiest done by having pandas-stubs declare a dependency on a package that provides stubs for matplotlib. I'm not actually aware of such a package — has the Pylance team published their matplotlib stubs on PyPI?

Second, it seems like there's some minor issue in timestamps.pyi. It's possible there's a real error there, but you could also add a type ignore for it for now.

Once these two issues are resolved, stubtest will be able to run more completely and will likely have a lot more to say.

Dr-Irv · 2022-07-06T11:58:40Z

First, mypy needs to be able to resolve the reference to matplotlib. This is easiest done by having pandas-stubs declare a dependency on a package that provides stubs for matplotlib. I'm not actually aware of such a package — has the Pylance team published their matplotlib stubs on PyPI?

It's "published" on https://github.com/microsoft/python-type-stubs , but I know those stubs are incomplete. And mypy won't read stubs unless they are installed.

With mypy, we use ignore_missing_imports to avoid those errors.

Second, it seems like there's some minor issue in timestamps.pyi. It's possible there's a real error there, but you could also add a type ignore for it for now.

We use python_version = "3.8" to suppress those messages. That's because datetime.isocalendar() changed its return type from a tuple to a named tuple in python 3.9.

Once these two issues are resolved, stubtest will be able to run more completely and will likely have a lot more to say.

Would it be possible for stubtest to observe our mypy settings in its analysis?

hauntsaninja · 2022-07-16T19:27:32Z

Got it, in that case you can use python -m mypy.stubtest pandas --concise --mypy-config-file pyproject.toml to make it work.

However, I don't really recommend reusing your mypy config like this, since it won't be reflective of what your users will be doing. That is, your users won't have matplotlib stubs unless you make them a dependency, so in practice users' type checkers will resolve all of those to Any.

hauntsaninja · 2022-07-16T19:31:18Z

Example output, in case it's useful to you: https://paste.ee/p/KObbQ

twoertwein · 2022-07-16T19:42:06Z

@hauntsaninja Does stubtest also work well when there are only very few pyi files? The pandas code itself has a few pyi files for cython code. It would be great to use stubtest to ensure that all public functions declared in pyi match the cython implementation.

hauntsaninja · 2022-07-16T19:49:03Z

Does stubtest also work well when there are only very few pyi files

On mypy master, it should. There was a bug I fixed recently where stubtest would not check submodules that did not have corresponding pyi files. This was fixed in python/mypy#13030 (should be released soon as part of mypy 0.970).

In general, if you wish to avoid errors about things that are missing from the stub and only wish to check consistency with the things that are there, stubtest supports an --ignore-missing-stub argument.

hauntsaninja · 2022-07-16T20:44:03Z

@twoertwein Looks like running stubtest against the main pandas repo finds a bunch of things, e.g. see pandas-dev/pandas#47756 , pandas-dev/pandas#47758 (there are more errors as well)

twoertwein · 2022-07-21T15:25:42Z

@Dr-Irv I opened a PR to use stubtest on the pyi files within pandas pandas-dev/pandas#47817

Obviosuly, stubtest will report many errors when comparing pandas-stubs with pandas. But as @hauntsaninja pointed out, stubtest can generate an allow/ignore list. As a starting point, we could have an unfortunately enormously long ignore list to at least prevent adding new stubs that are not compatible with the implementation (or at least be aware of it and add an exception to the list). Over time we can then work on making this list smaller. I think it will never be empty but we can definitely make it smaller :) This will especially be helpful for the cleaning efforts in #149 and #128.

twoertwein · 2022-07-31T14:12:56Z

Sorry for the stupid question @hauntsaninja How does stubtest need to be called on pandas or pandas-stubs:

python -m mypy.stubtest pandas-stubs --concise --ignore-missing-stub --mypy-config-file pyproject.toml throws many errors (for example that Series is not generic: it is generic in the stubs but not in the implementation)
python -m mypy.stubtest pandas --concise --mypy-config-file pyproject.toml cannot seem to find any stubs

hauntsaninja · 2022-08-01T05:19:59Z

After running pip install -e . in my copy of pandas-stubs (not sure if strictly necessary), python -m mypy.stubtest pandas --concise --mypy-config-file pyproject.toml works for me.
Note that it complains about "failed to find stubs" for modules for which stubs do not actually exist, like pandas.util.version, but if you scroll up you'll see some more useful errors.

twoertwein · 2022-08-01T23:30:02Z

Thanks, I think I forgot to install the stubs!

Fix pd.read_excel

1e9cbaa

hauntsaninja mentioned this pull request Jun 21, 2022

Support Pandas IO operation Return Types python/mypy#12999

Closed

Dr-Irv mentioned this pull request Jun 29, 2022

CI: use matplotlib stubs #48

Closed

hauntsaninja added 2 commits July 1, 2022 10:55

add a test

3aa5617

Merge remote-tracking branch 'upstream/main' into excel

8d576eb

Dr-Irv requested changes Jul 1, 2022

View reviewed changes

hauntsaninja added 2 commits July 2, 2022 00:06

fix merge, cr feedback

d8dc6f4

actually fix merge

f47c8e1

Dr-Irv merged commit 82107f6 into pandas-dev:main Jul 1, 2022

hauntsaninja deleted the excel branch July 1, 2022 19:02

hauntsaninja mentioned this pull request Jul 16, 2022

TYP: Use stubtest to ensure consistency with pyi files pandas-dev/pandas#47760

Closed

		@@ -1012,6 +1016,12 @@ def test_frame_getitem_isin() -> None:
		assert_type(df[df.index.isin([1, 3, 5])], "pd.DataFrame")


		def test_read_excel() -> None:

Uh oh!

Fix pd.read_excel #33

Fix pd.read_excel #33

Uh oh!

Conversation

hauntsaninja commented Jun 21, 2022

Uh oh!

Dr-Irv commented Jun 21, 2022

Uh oh!

hauntsaninja commented Jul 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dr-Irv Jul 1, 2022

Choose a reason for hiding this comment

Uh oh!

Dr-Irv commented Jul 1, 2022

Uh oh!

hauntsaninja commented Jul 1, 2022

Uh oh!

Dr-Irv commented Jul 1, 2022

Uh oh!

Dr-Irv commented Jul 1, 2022

Uh oh!

Dr-Irv commented Jul 3, 2022

Uh oh!

hauntsaninja commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dr-Irv commented Jul 6, 2022

Uh oh!

hauntsaninja commented Jul 16, 2022

Uh oh!

hauntsaninja commented Jul 16, 2022

Uh oh!

twoertwein commented Jul 16, 2022

Uh oh!

hauntsaninja commented Jul 16, 2022

Uh oh!

hauntsaninja commented Jul 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

twoertwein commented Jul 21, 2022

Uh oh!

twoertwein commented Jul 31, 2022

Uh oh!

hauntsaninja commented Aug 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

twoertwein commented Aug 1, 2022

Uh oh!

Uh oh!

hauntsaninja commented Jul 1, 2022 •

edited

Loading

hauntsaninja commented Jul 6, 2022 •

edited

Loading

hauntsaninja commented Jul 16, 2022 •

edited

Loading

hauntsaninja commented Aug 1, 2022 •

edited

Loading