AlenkaF commented on PR #51680:
URL: https://github.com/apache/arrow/pull/51680#issuecomment-6057855792
I have made some tests locally (stubgen-pyx 0.2.23, mypy 2.4.0 (stubgen),
pyright 1.1.414 and Cython 3.2.4)
on
- `lib.pyx`/`pyarrow.lib` and
- `_compute.pyx`/`pyarrow.compute`.
This message is super long, sorry, so I added a short summary table first:
**stubgen-pyx**
- (+) If using inline annotations we could automatically regenerate stubs on
code changes
- (+) stubgen-pyx keeps `@overload` in the generated stubs
- (−) Private classes are missing from the generated stub (e.g.
`_Weakrefable`).
The `--include-private` flag fixes that, but other internals then leak in
(e.g. `CFixedWidthType`)
- (−) Missing return and argument types need to be added in Cython (inline
annotations)
- (−) `**kwargs` annotation seems to be dropped from the stubs (didn't dive
into this)
Note: output file has to match the module (`lib.pyx` -> `pyarrow/lib.pyi`)
**stubgen --inspect-mode**
- (+) Good stubs created for the compute module
But for Cython files:
- (−) Produces draft stubs that would have to be updated and maintained by
hand
- (−) Adds an `import _cython_3_2_4` to the stubs, which can't be resolved
and changes with every Cython upgrade.
- (−) Module-level Cython functions (`cython_function_or_method`) are
written as variables (e.g. `null: _cython_3_2_4.cython_function_or_method`).
- (−) Adds `__init__` methods inherited at runtime, some with an erroneous
`@classmethod`.
| | stubgen-pyx | stubgen --inspect-mode |
|---|---|---|
| Properties | Kept as properties, no return type | Turned into writable
attributes typed `Incomplete` |
| Overloads | Kept if added to the source | Fake overloads built from
docstring signatures and examples |
| Docstrings | Can be added in full or skipped with `--exclude-docstrings` |
Parsed for signatures; stubs become noisy and can contain invalid syntax (see
below) |
| Dunders | Only the ones in the source | Also Cython internals
(`__reduce_cython__`, `__pyx_vtable__`, ...) |
| Compute kernels | Not possible (kernels are generated at import time in
`compute.py`); it does generates options classes which are needed! | All
kernels generated with argument names and keyword-only markers; options
parameters typed from their defaults (`bool`, `int`), but can cause false
positives (e.g. `q=0.5` -> `float`) |
Example of invalid syntax from docstring parsing (stubgen --inspect-mode):
```python
@classmethod
def __init__(cls, dense_union<a: fixed_size_binary[10] = ..., b: string =
...) -> Any: ...
```
<details>
<summary>Type-checking the generated lib stubs</summary>
Note: For inspect mode, the 10 lines with invalid syntax from docstring
parsing had to be patched first, otherwise mypy stops on the syntax errors.
| | stubgen-pyx | stubgen --inspect-mode |
|---|---:|---:|
| **Total** | **54** | **364** |
| Undefined names | 42 | 178 |
| Enum members annotated | — | 73 |
| `__init__` from docstring parsing | — | 51 |
| Overloads from docstring parsing | — | 57 |
| Incompatible signatures in the PyArrow API (e.g. `from_arrays`) | 6 | 4 |
| Other (imports, ...) | 6 | 1 |
Runing pyright with `typeCheckingMode = "basic"` (as currently configured in
`pyproject.toml`) gives the same undefined-name errors as mypy but misses the
incompatible signatures in the PyArrow API (see
`reportIncompatibleMethodOverride` default). With `standard` mode they are
reported together with one more: `Method "dictionary_encode" overrides class
"Array" in an incompatible manner`.
</details>
<details>
<summary>Mix of stubgen --inspect-mode and stubgen-pyx for the compute
module</summary>
I ran `stubgen --inspect-mode` on `pyarrow.compute` and stubgen-pyx on
`_compute.pyx`, and the two complement each other. The inspect stub
(`compute.pyi`) contains all the kernels, with correct signatures but missing
types, and re-exports the options classes (e.g. `ArraySortOptions`,
`AssumeTimezoneOptions`) from `pyarrow._compute`. Their definitions come from
the stubgen-pyx `_compute.pyi`. The two are connected by a plain import, so no
merging is needed.
`compute.pyi` itself has no errors in either mypy or pyright. pyright
reports only on the file it is run on, while mypy also reports errors in the
imported stubs: about 80 in `_compute.pyi`. Most of those come from the private
classes. The rest:
- A C type alias leaks into the stub (`CRegisterUdf`, 6 errors).
- `c_bool` in a public signature, it should be `bool`.
- A bug in the code is also found: `FunctionOptions.__eq__` raises
`TypeError` for a different type instead of returning `False`
(`pc.ScalarAggregateOptions() == 5`). `CacheOptions.__eq__` has the same
pattern.
</details>
<details>
<summary>Run type checkers on test_types.py</summary>
I have also used type checkers (`mypy --check-untyped-defs` and pyright on
standard mode) with generated stubs and have run them on `test_types.py`. I
found that:
- 1 of 19 `TypeError` from the test asserts has been found (19 out of 48
`pytest.raises` expect `TypeError`)
- 15 pyright/16 mypy `TypeError`'s have been found if I used inline
annotations in `types.pxi` similar to changes in
https://github.com/apache/arrow/pull/51681 (created them with an agent to have
a quick test, added 16 signatures, 2 overloads and 3 return types).
- pyright found 40 false positives due to generated `__init__.pyi` not
re-exporting the submodules (which is easily fixed)
- mypy found 6 false positives from the `sparse_factories`/`dense_factories`
loops
- approx 10 mypy/ 2 pyright errors from optional imports (dateutil, etc.)
</details>
<details>
<summary>Run type checkers on test_compute.py</summary>
I have left Claude to make a change in `_make_signature` and
`_wrap_function` that adds annotations to the sum kernel on runtime. The result
of running type checkers on `test_compute.py` with and without added sum
annotations on the mix of stubgen and stubgen-pyx generated stubs is as follows:
- generated stubs found 65 (mypy)/174 (pyright) errors,
- 3 real errors caught by both checkers, all structural: missing argument,
unexpected keyword and missing `indices`,
- about 50 false positives (without injected type annotations)
- Options parameters marked as required (20),
- types guessed from default values (30)
- one rule in `_make_signature` removed 20 false positives from Options
parameter, but at the cost of 1 real catch. Overloads could fix that but they
can't be injected in the stubs this way. A separate script would be needed for
that.
- Typing addition in `_wrap_function` works (adds stubs) and adds no false
positives. Annotations must be strings, because stubgen's inspect mode drops
runtime union objects.
We would also need to add annotations to option classes in `_compute.pyx`
but I have not tested yet if Cython keeps annotations at runtime.
</details>
I hope this is helpful. I am just learning about typing so I have no
experience on this field but feel the results are pretty good and would happily
work on the generated simple type stubs first.
What do other think?
Note: I used Claude heavily in the last, test_compute.py section, as I felt
it does not need a thorough look at right now but a fast test would do. On
other fronts I mainly used Claude for guidance, error summary and notes review).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
