Skip to content
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

Openpyxl typeshed errors #9836

Closed
tritemio opened this issue Mar 3, 2023 · 6 comments
Closed

Openpyxl typeshed errors #9836

tritemio opened this issue Mar 3, 2023 · 6 comments

Comments

@tritemio
Copy link

tritemio commented Mar 3, 2023

In an internal project, updating to 3.0.4.7 causes mypy in strict mode to generate the following error:

ocra.py:120: error: Value of type "_WorkbookChild" is not indexable  [index]
ocra.py:124: error: "_WorkbookChild" has no attribute "iter_rows"  [attr-defined]

The code for line 120 is:

...
wb = openpyxl.load_workbook(path)
ws = wb.active
header = [cell.value for cell in ws[1]]  # line 120
...

and for line 124:

ws.iter_rows(min_row=2)  # line 124

The same code checked using 3.0.4.6 passes all mypy checks in strick mode.

It seems like #9764 introduced a regression?

@srittau
Copy link
Collaborator

srittau commented Mar 6, 2023

#9764 actually made load_workbook() return a Workbook. Previously, it returned Any. The problem is that wb.active can return other workbook children (and even None), not only Worksheets. The easiest way to work around this problem is to use:

wb = openpyxl.load_workbook(path)
ws = wb.active
assert isinstance(ws, Worksheet)
header = [cell.value for cell in ws[1]]  # line 120

Cc @mynhardtburger

@Avasam
Copy link
Collaborator

Avasam commented Mar 7, 2023

+1 what @srittau said.
Even in #9511 I made load_workbook return a Workbook, the type comes from: https://foss.heptapod.net/openpyxl/openpyxl/-/blob/branch/3.1/openpyxl/reader/workbook.py#L31

@tritemio
Copy link
Author

tritemio commented Mar 8, 2023

Thanks for the quick answer. I'll apply the workaround to fix the code.

Maybe it could a good idea to write in the changelog that the type stubs are now more accurate and this may require changes to existing code bases that implicitly relied on wb.active to be Any. A link to the fix in this thread could also be useful.

@srittau
Copy link
Collaborator

srittau commented Mar 8, 2023

@tritemio Unfortunately, our changelogs are currently auto-generated from git commit messages, which is why they are a bit messy and we can't edit them retroactively.

@AlbertoPuritano
Copy link

AlbertoPuritano commented Mar 9, 2023

#9764 actually made load_workbook() return a Workbook. Previously, it returned Any. The problem is that wb.active can return other workbook children (and even None), not only Worksheets. The easiest way to work around this problem is to use:

wb = openpyxl.load_workbook(path)
ws = wb.active
assert isinstance(ws, Worksheet)
header = [cell.value for cell in ws[1]]  # line 120

Cc @mynhardtburger

Hello, apparently using the assert doesn't prevent mypy from reporting the error.. type hinting doesn't do much either, do you have any other solutions?

@srittau
Copy link
Collaborator

srittau commented Jun 12, 2023

I think the problems with wb.active have been explained. There isn't a better solution at the moment, until something like python/typing#566 becomes available.

@srittau srittau closed this as not planned Won't fix, can't repro, duplicate, stale Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants