[rok]

Rationale for this change

This is the next step in adding type annotations to PyArrow (#32609). It merges #49103 (internal type system stubs) and #49193 (core data structures) because the two depend on each other and can't be type checked separately.

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Was AI used for this PR?

In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.

PR code and description written by:

• Human
• AI

Reviewed before submission by:

• Human
• AI
• Not reviewed

• GitHub Issue: [Python][Annotations] Add internal types and Core Data Structures #55263

[github-actions]

⚠️ GitHub issue #55263 has been automatically assigned in GitHub to PR creator.

[AlenkaF]

I have run pytest pyarrow-stubs/tests, no errors found. I have looked through the tests also and tried to break some, everything looks good. I think the tests are easy to follow while still cover a lot of different angles.

I have also run mypy's stubtest with stubtest pyarrow.lib --concise --mypy-config-file pyproject.toml. All of the reported discrepancies are cases of "... is not present in stub" with majority of them being stubs that are not yet included in this PR (scalars, io, etc.). Part of these are also Type_* stubs, for example pyarrow.lib.Type_UINT8. As we already have TypesEnum defined here, might it make sense to also include Type_* stubs now? (I am not inclined towards adding them, just wondering).

Besides the reports mentioned above which will be fixed in subsequent PRs, stubtest did found one bug in pyarrow:

pyarrow.lib.Decima32Array is not present in stub

Note missing "L" in "Decimal". I will work on the fix for this now and would suggest to add it as part of this PR. Can commit directly, if that is ok.

[tadeja]

I've run pre-commit run --hook-stage manual python-stubs --all-files which currently uses pyarrow==26.0.0 wheel modules in site-packages/pyarrow, and got all passing (Python Stubs Lint (ruff), Python Stubs Type Check (mypy/pyright/ty)) - except for ruff-format which requires 10 files to be reformatted, fyi @rok
Python Stubs Format (ruff)...............................................Failed

Running on editable dev build 27, with pyright==1.1.414 in my conda env, I also hit errors reportMissingImports and warnings reportMissingModuleSource - Suggested changes by @AlenkaF sound good and seem to work fine for me locally.

[rok]

Thanks for continuing to push this forward Rok! I think this PR is a great next step and is quite clear to review.

I have checked out the PR locally and started with running type checkers (mypy, pyright and ty). mypy and ty emit no errors or warnings, but pyright does emit reportMissingImports and reportMissingModuleSource which get fixed with adding _compute.pyi placeholder (similar to existing io.pyi) and reportMissingModuleSource = false under [tool.pyright] in pyproject.toml.

The reason for these warnings and errors in pyright is that locally I have editable pyarrow install where scikit-build-core puts the .so files in a separate folder (not python/pyarrow) without __init__.py and pyright can't find them. As we do not need the compiled modules for the type checkers, I think it is fine to use reportMissingModuleSource = false.

I propose to push the changes directly if we agree the additions makes sense.

That makes sense and please go for it. I think we should aim for pyproject.toml setting to be as close to the state we want to maintain as possible. So far I've tried to exclude modules that are not covered and ignore as few things as possible. reportMissingModuleSource = false seems good, we will want to challenge it when we have complete coverage.

Part of these are also Type_* stubs, for example pyarrow.lib.Type_UINT8. As we already have TypesEnum defined here, might it make sense to also include Type_* stubs now? (I am not inclined towards adding them, just wondering).

Huh. I've never noticed these were missing. Given we're not really using them as types I'd skip them if we can.

Besides the reports mentioned above which will be fixed in subsequent PRs, stubtest did found one bug in pyarrow:

pyarrow.lib.Decima32Array is not present in stub

Note missing "L" in "Decimal". I will work on the fix for this now and would suggest to add it as part of this PR. Can commit directly, if that is ok.

Huh. Let's do the fix as separate PR, but also include a fix here? (And we'll just rebase over the actual fix on main)

I've run pre-commit run --hook-stage manual python-stubs --all-files which currently uses pyarrow==26.0.0 wheel modules in site-packages/pyarrow, and got all passing (Python Stubs Lint (ruff), Python Stubs Type Check (mypy/pyright/ty)) - except for ruff-format which requires 10 files to be reformatted, fyi @rok Python Stubs Format (ruff)...............................................Failed

I'm not sure we want (can) bring in ruff as a CI dependency. However we should autoformat before we merge, so we don't do it later and change history for no good reason. So I propose one of you runs the ruff formatter and commits the change into this branch.

[AlenkaF]

reportMissingModuleSource = false seems good, we will want to challenge it when we have complete coverage.

I think reportMissingModuleSource will have to stay due to local editable installs. Otherwise I agree, we should ignore as few things as possible.
Will push the commit 👍

Huh. Let's do the fix as separate PR, but also include a fix here? (And we'll just rebase over the actual fix on main)

Sounds good. Will open an issue and a PR and add a fix here also.

[AlenkaF]

Part of these are also Type_* stubs, for example pyarrow.lib.Type_UINT8. As we already have TypesEnum defined here, might it make sense to also include Type_* stubs now? (I am not inclined towards adding them, just wondering).

Huh. I've never noticed these were missing. Given we're not really using them as types I'd skip them if we can.

Yeah, they are internal and exposed through TypesEnum which are added to the stubs here. For stubtest we can later add them to allowlist. For the type checkers we will need to work around them once lib stubs loose their def __getattr__(name: str) -> Any: .... Maybe we can revisit this topic later (maybe we see it's easier to simply add them to the stubs)

[tadeja]

... ruff-format which requires 10 files to be reformatted, fyi @rok Python Stubs Format (ruff)...............................................Failed

I'm not sure we want (can) bring in ruff as a CI dependency. However we should autoformat before we merge, so we don't do it later and change history for no good reason. So I propose one of you runs the ruff formatter and commits the change into this branch.

Ok, I'll do that reformatting in a bit.

[AlenkaF]

I have started with reviewing pyarrow tests with running mypy --check-untyped-defs --warn-unused-ignores pyarrow/tests/test_* and looking at the ignores. The stubs seem to have changed since the last time type checkers were run on pyarrow tests, example:

click for output

$ mypy --check-untyped-defs --warn-unused-ignores pyarrow/tests/test_array.py 
pyarrow/tests/test_array.py:21: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:22: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:25: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:80: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:790: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:986: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:1300: error: List item 3 has incompatible type "None"; expected "int"  [list-item]
pyarrow/tests/test_array.py:1302: error: List item 1 has incompatible type "None"; expected "list[tuple[bytes, int]]"  [list-item]
pyarrow/tests/test_array.py:1313: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:1320: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:1324: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:1348: error: List item 0 has incompatible type "None"; expected "list[tuple[bytes, int]]"  [list-item]
pyarrow/tests/test_array.py:1348: error: List item 1 has incompatible type "None"; expected "list[tuple[bytes, int]]"  [list-item]
pyarrow/tests/test_array.py:1362: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:2513: error: Need type annotation for "buffers" (hint: "buffers: list[<type>] = ...")  [var-annotated]
pyarrow/tests/test_array.py:2539: error: "object" has no attribute "dtype"  [attr-defined]
pyarrow/tests/test_array.py:2541: error: Value of type "object" is not indexable  [index]
pyarrow/tests/test_array.py:2542: error: Value of type "object" is not indexable  [index]
pyarrow/tests/test_array.py:2543: error: Value of type "object" is not indexable  [index]
pyarrow/tests/test_array.py:2885: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:2917: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:2938: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:2939: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:3183: error: Value of type "object" is not indexable  [index]
pyarrow/tests/test_array.py:3299: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:3720: error: Incompatible types in assignment (expression has type "None", variable has type "ndarray[tuple[Any, ...], dtype[Any]]")  [assignment]
pyarrow/tests/test_array.py:3729: error: Value of type "object" is not indexable  [index]
pyarrow/tests/test_array.py:3740: error: Value of type "object" is not indexable  [index]
pyarrow/tests/test_array.py:3817: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:4004: error: Incompatible types in assignment (expression has type "MyArrayInvalid", variable has type "MyArray")  [assignment]
pyarrow/tests/test_array.py:4017: error: Incompatible types in assignment (expression has type "MyArray2", variable has type "MyArray")  [assignment]
pyarrow/tests/test_array.py:4031: error: Incompatible types in assignment (expression has type "MyArray3", variable has type "MyArray")  [assignment]
pyarrow/tests/test_array.py:4095: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:4099: error: Unused "type: ignore" comment  [unused-ignore]
pyarrow/tests/test_array.py:4315: error: List item 3 has incompatible type "None"; expected "int"  [list-item]
pyarrow/tests/test_array.py:4319: error: List item 5 has incompatible type "None"; expected "int"  [list-item]
pyarrow/tests/test_array.py:4324: error: List item 2 has incompatible type "None"; expected "int"  [list-item]
pyarrow/tests/test_array.py:4324: error: List item 4 has incompatible type "None"; expected "int"  [list-item]
pyarrow/tests/test_array.py:4395: error: List item 1 has incompatible type "None"; expected "int"  [list-item]
pyarrow/tests/test_array.py:4407: error: List item 0 has incompatible type "None"; expected "int"  [list-item]
Found 40 errors in 1 file (checked 1 source file)

Type checkers are currently configured to skip python/pyarrow folder (too much noise) so the changes are not caught. Maybe we could start with adding mypy --check-untyped-defs --warn-unused-ignores pyarrow/tests/test_* for specific files that can already be type checked.

I propose to temporarily add the mypy command explicitly naming files which bypasses the exclude setting to ci/scripts/python_test_type_annotations.sh. Will also push the updates of ignores in the tests once I go through the ones connected to this PR.

[AlenkaF]

Another thing: I can't run pyright on specific files inside an excluded dir. I guess I would need to be running the checks with pyright only locally (with the pyarrow exclude temporarily removed) until the pyarrow/ exclude in pyproject.toml is removed.

[AlenkaF]

Oh, I was too fast here. Unfortunately def __getattr__(name: str) -> Any: ... in python/pyarrow-stubs/pyarrow/__init__.pyi makes anything in form of pa.* be Any making lots of correct ignores (in with pytest.raises ... blocks) falsely reported as unused.

Would it make sense to remove # type: ignores from the pyarrow tests for now and add type checking of the tests (with the ignores) after the stubs are in?

I will leave this for discussion and start with reading the stubs and comparing them with the APIs, starting with _types.pyi first.

[rok]

Ignore statements not being needed is more likely due to changes to typecheckers than due to changes in stubs. Or they might be needed once we import other parts of the library. Also note camel cased (e.g. reportArgumentType) ignores are likely coming from pyright. So if mypy thinks it's a redundant ignore you might want to check pyright too.

[rok]

Oh, I was too fast here. Unfortunately def __getattr__(name: str) -> Any: ... in python/pyarrow-stubs/pyarrow/__init__.pyi makes anything in form of pa.* be Any making lots of correct ignores (in with pytest.raises ... blocks) falsely reported as unused.

Would it make sense to remove # type: ignores from the pyarrow tests for now and add type checking of the tests (with the ignores) after the stubs are in?

Let's rather remove def __getattr__(name: str) -> Any: ... at least for development work? Otherwise we'll just need to add ignores again.

[AlenkaF]

OK, agree, when reviewing pyarrow/tests we should remove def __getattr__(name: str) -> Any: ... from python/pyarrow-stubs/pyarrow/__init__.pyi so we keep the ignores and run mypy and pyright (with updated pyproject.toml) so both pass 👍

I plan to do that later and am focusing on reading the stubs currently.

[tadeja]

Ehhh... I've fallen into this interesting rabbit hole - There are over 1.5k mypy errors Module not callable in pyarrow/tests because of the change from upstream's zen-xu/pyarrow-stubs having subfolder __lib_pxi to current PR's flat layout without a subfolder (Past discussion rok#45 (comment) was about the naming, but not really why removing subfolder __lib_pxi)

% mypy --check-untyped-defs --warn-unused-ignores pyarrow/tests/test_convert_builtin.py 
pyarrow/tests/test_convert_builtin.py:78: error: Module not callable  [operator]
pyarrow/tests/test_convert_builtin.py:79: error: Module not callable  [operator]
...
pyarrow/tests/test_convert_builtin.py:168: error: Module not callable  [operator]
...
Found 321 errors in 1 file (checked 1 source file)

% mypy --check-untyped-defs --warn-unused-ignores pyarrow/tests/test_*.py
Found 1866 errors in 35 files (checked 44 source files)
% ... | grep -c  "Module not callable"
1562

test_convert_builtin.py:168 : pa.array(...

A stub pyarrow/array.pyi declares pyarrow.array submodule that doesn't exist at runtime because array.pxi gets compiled into lib. Then pa.array, pa.table or pa.scalar can resolve to stub modules instead of functions.
The count of mypy errors may be even higher, depending on module processing order and mypy cache. (optionally run rm -rf .mypy_cache)

So I propose we reintroduce subfolder __lib_pxi/ (or renamed subfolder _lib_pxi/ or just _lib/) in this PR (for files: array.pyi, builder.pyi, config.pyi, device.pyi, error.pyi, io.pyi, memory.pyi, scalar.pyi, table.pyi, tensor.pyi).
Locally that seems to resolve all 1.5k Module not callable errors and still passes type checkers' tests for me.

[tadeja]

Just to note: with commit eda3425 stubs array, builder, config, device, error, io, memory, scalar, table, tensor got moved into pyarrow/__lib/ subfolder/subpackage for mypy to not regard pa.array/pa.table/pa.scalar as stub modules anymore and so more than 1500 errors Module not callable in pyarrow/tests no longer show up.

[jorisvandenbossche]

@tadeja I suppose the _types.pyi also needs to be moved into the subdirectory (and can then be renamed to types.pyi)?

[tadeja]

@jorisvandenbossche oh, yes, I think _types.pyi and _ipc.pyi both still need to be moved but there are some extra steps needed (and @AlenkaF has some work in progress on types at the moment, and I may wait for that)

[jorisvandenbossche]

What is the point of having both the NullableCollection and Iterable? (the collection seems a strict subset of Iterable)