Merge `.pyi` type stubs inline #563

dblock · 2023-11-01T17:07:02Z

Description

Merge .pyi files inline for those files that were previously covered by mypy. Many Any that should not be, but it gets us started to have a fully typed library that is edited at the same time as code. This should greatly improve code clarity, maintainability, and enable better static analysis.

I wasn't sure whether this is a good idea, so I found some other projects merging stubs.

Merge type stubs for torch.optim pytorch/pytorch#102428 says that stubs mask the existence of methods, which can lead to spurious type errors in child classes of these classes, and turns off mypy for functions whose type hints are defined only in the stubs, since mypy doesn't run on functions/methods without inline type hints.
First draft of DEP for static-ish typechecking for Django django/deps#65

Issues Resolved

Closes #536.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: dblock <[email protected]>

codecov · 2023-11-01T17:21:34Z

Codecov Report

Merging #563 (a97845f) into main (0d8a23d) will increase coverage by 0.58%.
The diff coverage is 96.78%.

@@            Coverage Diff             @@
##             main     #563      +/-   ##
==========================================
+ Coverage   71.10%   71.69%   +0.58%     
==========================================
  Files          85       87       +2     
  Lines        7774     7875     +101     
==========================================
+ Hits         5528     5646     +118     
+ Misses       2246     2229      -17

Files	Coverage Δ
opensearchpy/__init__.py	`100.00% <100.00%> (ø)`
opensearchpy/_async/client/_patch.py	`48.14% <100.00%> (+1.99%)`	⬆️
opensearchpy/_async/client/client.py	`100.00% <100.00%> (ø)`
opensearchpy/_async/client/cluster.py	`52.32% <100.00%> (+0.56%)`	⬆️
opensearchpy/_async/client/dangling_indices.py	`56.25% <100.00%> (+2.91%)`	⬆️
opensearchpy/_async/client/features.py	`77.77% <100.00%> (+2.77%)`	⬆️
opensearchpy/_async/client/indices.py	`46.47% <100.00%> (+0.25%)`	⬆️
opensearchpy/_async/client/ingest.py	`52.00% <100.00%> (+2.00%)`	⬆️
opensearchpy/_async/client/nodes.py	`65.00% <100.00%> (+1.84%)`	⬆️
opensearchpy/_async/client/plugins.py	`95.00% <100.00%> (+1.25%)`	⬆️
... and 74 more

Signed-off-by: dblock <[email protected]>

dblock · 2023-11-03T19:23:29Z

@saimedhi this is ready for review, FYI

saimedhi

@dblock, I've reviewed the PR, and from what I understand, API types are added automatically through the generator, while for tests and other files, we add them manually. I've made a few requested changes, and if you agree with them, please make the changes. Thank you for your contribution!

saimedhi · 2023-11-04T12:43:59Z

benchmarks/bench_async.py

@@ -71,34 +71,34 @@ async def test_async(client_count=1, item_count=1):
    await asyncio.gather(*[client.close() for client in clients])


-def test(item_count=1, client_count=1):
+def test(item_count: int = 1, client_count: int = 1) -> None:


type annotation missing for "test_async" function in this file.

There's another 1000 errors from the missing files that were not previously checked by mypy, including benchmarks and tests. Could we merge this one and I'll work on the rest?

opensearchpy/__init__.py

benchmarks/bench_async.py

benchmarks/bench_info_sync.py

benchmarks/bench_sync.py

Signed-off-by: dblock <[email protected]>

dblock · 2023-11-06T15:03:30Z

@saimedhi Thanks for the CR! I edited the description to say "Merge .pyi files inline for those files that were previously covered by mypy." I'd prefer we merge this as an intermediate step, but lmk if you feel strongly otherwise. Opened #564 to add types to the rest of the code.

The new opensearch-py 2.4.1 added mypy stubs in opensearch-project/opensearch-py#563 and they detected an error in our example (and they cause mypy error when the new version is used)

* Merged types into .py code. Signed-off-by: dblock <[email protected]> * Fix: nox -rs generate. Signed-off-by: dblock <[email protected]> * Updated CHANGELOG. Signed-off-by: dblock <[email protected]> * Use lowest common python version for lint. Signed-off-by: dblock <[email protected]> * Fix: don't typeshed. Signed-off-by: dblock <[email protected]> * Removed unneeded comment. Signed-off-by: dblock <[email protected]> * Simplify OPENSEARCH_URL. Signed-off-by: dblock <[email protected]> * Fix: positional ignore_status used as chunk_size. Signed-off-by: dblock <[email protected]> * Fix: parse version string. Signed-off-by: dblock <[email protected]> * Remove future annotations for Python 3.6. Signed-off-by: dblock <[email protected]> * Fix: types in documentation. Signed-off-by: dblock <[email protected]> * Improve CHANGELOG text. Signed-off-by: dblock <[email protected]> * Re-added missing separator. Signed-off-by: dblock <[email protected]> * Remove duplicate licenses. Signed-off-by: dblock <[email protected]> * Get rid of Optional[Any]. Signed-off-by: dblock <[email protected]> * Fix docs with AsyncOpenSearch. Signed-off-by: dblock <[email protected]> * Fix: undo comment. Signed-off-by: dblock <[email protected]> --------- Signed-off-by: dblock <[email protected]> Signed-off-by: roma2023 <[email protected]>

The new opensearch-py 2.4.1 added mypy stubs in opensearch-project/opensearch-py#563 and they detected an error in our example (and they cause mypy error when the new version is used) GitOrigin-RevId: dffe04ada4c05a3e29422f6ea92625c6e040bf10

dblock added 2 commits November 1, 2023 12:49

Merged types into .py code.

6b0518b

Signed-off-by: dblock <[email protected]>

Fix: nox -rs generate.

9a57c78

Signed-off-by: dblock <[email protected]>

dblock requested review from VachaShah, harshavamsi, axeoman, deztructor, Shephalimittal, saimedhi and florianvazelle as code owners November 1, 2023 17:07

Updated CHANGELOG.

dc3b013

Signed-off-by: dblock <[email protected]>

dblock marked this pull request as draft November 1, 2023 17:11

Use lowest common python version for lint.

491e00f

Signed-off-by: dblock <[email protected]>

dblock added 6 commits November 1, 2023 13:23

Fix: don't typeshed.

b03ae32

Signed-off-by: dblock <[email protected]>

Removed unneeded comment.

7df36cd

Signed-off-by: dblock <[email protected]>

Simplify OPENSEARCH_URL.

a98d408

Signed-off-by: dblock <[email protected]>

Fix: positional ignore_status used as chunk_size.

df6133e

Signed-off-by: dblock <[email protected]>

Fix: parse version string.

64251f0

Signed-off-by: dblock <[email protected]>

Remove future annotations for Python 3.6.

05b919a

Signed-off-by: dblock <[email protected]>

dblock marked this pull request as ready for review November 1, 2023 20:06

dblock changed the title ~~Merge .pyi files inline.~~ Merge .pyi stubs inline. Nov 1, 2023

dblock mentioned this pull request Nov 1, 2023

[PROPOSAL] Merge .pyi types inline #536

Closed

dblock added 2 commits November 1, 2023 16:40

Fix: types in documentation.

122d0ad

Signed-off-by: dblock <[email protected]>

Improve CHANGELOG text.

151c026

Signed-off-by: dblock <[email protected]>

dblock changed the title ~~Merge .pyi stubs inline.~~ Merge .pyi type stubs inline Nov 1, 2023

dblock added 2 commits November 1, 2023 16:44

Re-added missing separator.

92db922

Signed-off-by: dblock <[email protected]>

Remove duplicate licenses.

2e88194

Signed-off-by: dblock <[email protected]>

dblock force-pushed the merge-pyi branch from d2e6c69 to 2e88194 Compare November 1, 2023 20:52

dblock added 2 commits November 1, 2023 16:59

Get rid of Optional[Any].

96fcc3e

Signed-off-by: dblock <[email protected]>

Fix docs with AsyncOpenSearch.

0e13cc9

Signed-off-by: dblock <[email protected]>

dblock mentioned this pull request Nov 2, 2023

[BUG] resources ported from opensearch-dsl-py causing mypy errors #402

Closed

saimedhi reviewed Nov 6, 2023

View reviewed changes

Fix: undo comment.

a97845f

Signed-off-by: dblock <[email protected]>

dblock mentioned this pull request Nov 6, 2023

[FEATURE] Add mypy coverage outside fo opensearchpy/* #564

Closed

saimedhi approved these changes Nov 6, 2023

View reviewed changes

saimedhi merged commit dcb79cc into opensearch-project:main Nov 6, 2023
54 checks passed

dblock deleted the merge-pyi branch November 6, 2023 21:21

potiuk mentioned this pull request Nov 16, 2023

Fix mypy index property access in opensearch example apache/airflow#35691

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge `.pyi` type stubs inline #563

Merge `.pyi` type stubs inline #563

dblock commented Nov 1, 2023 •

edited

Loading

codecov bot commented Nov 1, 2023 •

edited

Loading

dblock commented Nov 3, 2023

saimedhi left a comment •

edited

Loading

saimedhi Nov 4, 2023

dblock Nov 6, 2023

dblock commented Nov 6, 2023 •

edited

Loading

Merge .pyi type stubs inline #563

Merge .pyi type stubs inline #563

Conversation

dblock commented Nov 1, 2023 • edited Loading

Description

Issues Resolved

codecov bot commented Nov 1, 2023 • edited Loading

Codecov Report

dblock commented Nov 3, 2023

saimedhi left a comment • edited Loading

Choose a reason for hiding this comment

saimedhi Nov 4, 2023

Choose a reason for hiding this comment

dblock Nov 6, 2023

Choose a reason for hiding this comment

dblock commented Nov 6, 2023 • edited Loading

Merge `.pyi` type stubs inline #563

Merge `.pyi` type stubs inline #563

dblock commented Nov 1, 2023 •

edited

Loading

codecov bot commented Nov 1, 2023 •

edited

Loading

saimedhi left a comment •

edited

Loading

dblock commented Nov 6, 2023 •

edited

Loading