Additional type hints added to stub `openpyxl` by ArnabRollin · Pull Request #9487 · python/typeshed

ArnabRollin · 2023-01-10T11:50:18Z

Reason for change

The VS Code Python static type-checker does not recognise

Here is an example:

from openpyxl import load_workbook

wb = load_workbook("myxl.xlsx") # Error: Function 'load_workbook' is partially unknown

Mypy doesn't seem to find the types-openpyxl package

(Working-with-Excel) -> pipenv install types-openpyxl

Installing types-openpyxl...
Pipfile.lock (43417e) out of date, updating to (83c742)...
Locking [packages] dependencies...
Locking [dev-packages] dependencies...
Updated Pipfile.lock (c868c9d2bef5b5f5f3c9b17280f5f7a15a9a9522fd50d8cb558d1eb0e083c742)!
Installing dependencies from Pipfile.lock (83c742)...
To activate this project's virtualenv, run pipenv shell.
Alternatively, run a command inside the virtualenv with pipenv run.

(Working-with-Excel) -> mypy app.py

app.py:1: error: Library stubs not installed for "openpyxl"  [import]
app.py:1: note: Hint: "python3 -m pip install types-openpyxl"
app.py:1: note: (or run "mypy --install-types" to install all missing stub packages)
app.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports

Please point out if I am wrong anywhere.

Updated classes `Workbook`, `Worksheet`, `Cell`. Type checking works in VS Code

Avasam · 2023-01-10T17:31:59Z

(I'm not a typeshed maintainer, but I contribute a lot here)
The VS Code Python static type-checker, named "Pylance", uses https://github.com/microsoft/pyright behind the scene. The reason you get partially unknown errors is because you're using some strict typing settings.

There's a big semantic difference between "Unknown" and "Any". One says to be careful because the type isn't checked. The other says you can truly use anything. Nowadays typeshed uses Any as an escape hatch to say "we can't accurately represent the type under the current type system".
mypy doesn't have an Unknown pseudo-type, it just uses Any.
That's also why, for semantic reasons, in typeshed we prefer to leave parameters and return types untyped. Or otherwise use the alias Incomplete (from _typeshed import Incomplete). See https://github.com/python/typeshed/blob/main/CONTRIBUTING.md#incomplete-stubs

To fix the issue on pyright/pylance. You could:

configure Pylance to not use strict typing.
use a config file and disable specific checks (https://github.com/microsoft/pyright/blob/main/docs/configuration.md#type-check-diagnostics-settings)
Suppress the error with # pyright: ignore[reportUnknownArgumentType,reportUnknownParameterType] until the stub is more complete
Complete the parts of the stub that you need with accurate types

To resolve the issue with mypy. Since you already installed types-openpyxl, make sure that you are using mypy from your virtual environment. I am not personally familiar with pipenv, but it's possible that mypy on your path isn't pointing to the one in your project's environment. You can try running pipenv install mypy and python3 -m mypy app.py.

pyright comes with a copy of typeshed's third-party stubs. mypy doesn't and you have to install them manually.
pyright also comes with a copy of https://github.com/microsoft/python-type-stubs/

openpyxl is also very special. All those class members using descriptors are actually properties in disguise. With runtime type-checking and type coercion. I'm currently working on merging the stubs from https://github.com/microsoft/python-type-stubs/tree/main/openpyxl

ArnabRollin · 2023-01-11T12:36:51Z

The reason you get partially unknown errors is because you're using some strict typing settings.

I usually like to put my code in strict mode. But, when I found the type stubs, I thought they were a bit incomplete... So I updated these.

AlexWaygood · 2023-01-11T17:00:04Z

But, when I found the type stubs, I thought they were a bit incomplete... So I updated these.

They are indeed :)

And contributions are very welcome! Thank you for the PR!

github-actions · 2023-01-11T17:18:38Z

Diff from mypy_primer, showing the effect of this PR on open source code:

pandas (https://github.com/pandas-dev/pandas)
+ pandas/io/excel/_openpyxl.py:73: error: Argument 1 to "load_workbook" has incompatible type "IO[bytes]"; expected "str"  [arg-type]
+ pandas/io/excel/_openpyxl.py:101: error: Argument 1 to "save" of "Workbook" has incompatible type "IO[bytes]"; expected "str"  [arg-type]
+ pandas/io/excel/_openpyxl.py:554: error: Argument 1 to "load_workbook" has incompatible type "Union[Union[str, PathLike[str]], ReadBuffer[bytes]]"; expected "str"  [arg-type]

AlexWaygood

Thanks! There's some good stuff in here, but I'm afraid there are also quite a few errors that we need to sort out. Remarks below:

AlexWaygood · 2023-01-11T22:14:39Z


-def get_type(t, value): ...
-def get_time_format(t): ...
+def get_type(t: Any, value: Any) -> Any: ...


t looks like it has to be a class of some kind, due to this line here, where it's used as a key in a dictionary that only has classes as keys: https://foss.heptapod.net/openpyxl/openpyxl/-/blob/branch/3.0/openpyxl/cell/cell.py#L73

value could be literally any object, so it's better to use object here rather than Any. Any is an "escape hatch" in the type system for things that are truly inexpressible. It's very unsafe, so it's better to use object whenever possible.

The function always either returns a str or None.

Suggested change

def get_type(t: Any, value: Any) -> Any: ...

def get_type(t: type, value: object) -> str | None: ...

AlexWaygood · 2023-01-11T22:18:44Z

-def get_type(t, value): ...
-def get_time_format(t): ...
+def get_type(t: Any, value: Any) -> Any: ...
+def get_time_format(t: Any) -> Any: ...


t is clearly expected to be a datetime object, due to this TIME_FORMATS.get(t) call here (TIME_FORMATS only has datetime objects as keys): https://foss.heptapod.net/openpyxl/openpyxl/-/blob/branch/3.0/openpyxl/cell/cell.py#L78

The function always returns the value that's associated with the key passed in, and the values of TIME_FORMATS are all strs. So the function always returns a str.

Suggested change

def get_time_format(t: Any) -> Any: ...

def get_time_format(t: datetime) -> str: ...

AlexWaygood · 2023-01-12T14:03:08Z

+        worksheet: Worksheet,
+        row: int | None = ...,
+        column: int | None = ...,
+        value: str | float | int | datetime | None = ...,


AlexWaygood · 2023-01-12T14:03:44Z

+    def check_error(self, value: Any) -> Any | str: ...
    @property
-    def value(self): ...
+    def value(self) -> str | float | int | datetime | None: ...


AlexWaygood · 2023-01-12T14:03:54Z

+    def value(self) -> str | float | int | datetime | None: ...
    @value.setter
-    def value(self, value) -> None: ...
+    def value(self, value: str | float | int | datetime | None) -> None: ...


AlexWaygood · 2023-01-12T15:49:41Z

+    def delete_cols(self, idx: int, amount: int = ...) -> None: ...
+    def move_range(self, cell_range: CellRange, rows: int = ..., cols: int = ..., translate: bool = ...) -> None: ...
+    @property
+    def print_title_rows(self) -> Any: ...


Please leave unannotated if you don't know the type.

AlexWaygood · 2023-01-12T15:49:52Z

+    def print_title_rows(self, rows: int) -> None: ...
    @property
-    def print_title_cols(self): ...
+    def print_title_cols(self) -> Any: ...


Please leave unannotated if you don't know the type.

AlexWaygood · 2023-01-12T15:49:59Z

+    def print_titles(self) -> Any: ...
    @property
-    def print_area(self): ...
+    def print_area(self) -> Any: ...


Please leave unannotated if you don't know the type.

AlexWaygood · 2023-01-12T15:50:41Z

+    def print_area(self) -> Any: ...
    @print_area.setter
-    def print_area(self, value) -> None: ...
+    def print_area(self, value: str) -> None: ...


This is incorrect, it also accepts an iterable of strings: https://foss.heptapod.net/openpyxl/openpyxl/-/blob/branch/3.0/openpyxl/worksheet/worksheet.py#L883-891

AlexWaygood · 2023-01-12T15:51:31Z

+    def print_title_cols(self) -> Any: ...
    @print_title_cols.setter
-    def print_title_cols(self, cols) -> None: ...
+    def print_title_cols(self, cols: int) -> None: ...


This is incorrect, it accepts str | None, but not int: https://foss.heptapod.net/openpyxl/openpyxl/-/blob/branch/3.0/openpyxl/worksheet/worksheet.py#L855-863

mynhardtburger · 2023-02-18T19:30:30Z

@ArnabRollin are you still working on this, or can I offer my help to implement the change requests?

AlexWaygood · 2023-02-19T14:39:35Z

@mynhardtburger would you like to pick up where @ArnabRollin picked off? You'd be very welcome to! (@ArnabRollin, you'll be listed as a co-author, of course.)

mynhardtburger · 2023-02-19T15:47:35Z

@AlexWaygood I would like to try...
I assume the process would be to fork ArnabRollin's work, work through your suggested changes and then submit a new PR?

This is my first time contributing to a public project.

AlexWaygood · 2023-02-19T15:49:36Z

@AlexWaygood I would like to try... I assume the process would be to fork ArnabRollin's work, work through your suggested changes and then submit a new PR?

This is my first time contributing to a public project.

Yes, that would be the process! :)

Avasam · 2023-02-19T19:51:34Z

@AlexWaygood I would like to try... I assume the process would be to fork ArnabRollin's work, work through your suggested changes and then submit a new PR?

This is my first time contributing to a public project.

If you do, please tag me in the new PR so I can update the description in #9511

ArnabRollin and others added 3 commits January 10, 2023 16:56

Updated type hints

b68f9eb

Updated classes `Workbook`, `Worksheet`, `Cell`. Type checking works in VS Code

Merge branch 'python:main' into main

948d5f6

[pre-commit.ci] auto fixes from pre-commit.com hooks

ad01e57

This comment has been minimized.

Sign in to view

Merge branch 'main' into main

dd58853

pre-commit-ci Bot and others added 2 commits January 11, 2023 17:02

[pre-commit.ci] auto fixes from pre-commit.com hooks

ed44b07

Add back missing import

933b9f7

AlexWaygood self-requested a review January 11, 2023 22:33

AlexWaygood requested changes Jan 12, 2023

View reviewed changes

Avasam mentioned this pull request Jan 12, 2023

Merge openpyxl stubs from microsoft/python-type-stubs #9511

Closed

parched mentioned this pull request Feb 6, 2023

openpyxl: add a few type annotations #9681

Closed

ArnabRollin closed this Feb 19, 2023

mynhardtburger mentioned this pull request Feb 19, 2023

Additional type hints added to stub openpyxl #9764

Merged

	def get_type(t: Any, value: Any) -> Any: ...
	def get_type(t: type, value: object) -> str \| None: ...

	def get_time_format(t: Any) -> Any: ...
	def get_time_format(t: datetime) -> str: ...

	value: str \| float \| int \| datetime \| None = ...,
	value: str \| float \| datetime \| None = ...,

	def value(self) -> str \| float \| int \| datetime \| None: ...
	def value(self) -> str \| float \| datetime \| None: ...

	def value(self, value: str \| float \| int \| datetime \| None) -> None: ...
	def value(self, value: str \| float \| datetime \| None) -> None: ...

Uh oh!

Conversation

ArnabRollin commented Jan 10, 2023

Reason for change

Uh oh!

This comment has been minimized.

Avasam commented Jan 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArnabRollin commented Jan 11, 2023

Uh oh!

AlexWaygood commented Jan 11, 2023

Uh oh!

github-actions Bot commented Jan 11, 2023

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mynhardtburger commented Feb 18, 2023

Uh oh!

AlexWaygood commented Feb 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mynhardtburger commented Feb 19, 2023

Uh oh!

AlexWaygood commented Feb 19, 2023

Uh oh!

Avasam commented Feb 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Avasam commented Jan 10, 2023 •

edited

Loading

AlexWaygood commented Feb 19, 2023 •

edited

Loading