opensearch-project / opensearch-py

Python Client for OpenSearch
https://opensearch.org/docs/latest/clients/python/
Apache License 2.0
338 stars 170 forks source link

[PROPOSAL] Merge .pyi types inline #536

Closed dblock closed 10 months ago

dblock commented 11 months ago

What/Why

What are you proposing?

We have the codebase littered with .pyi files. Is it time to merge types into Python code?

What problems are you trying to solve?

Updating types in multiple places is error prone.

What is the developer experience going to be?

No more .pyi files.

Djcarrillo6 commented 11 months ago

Hi @dblock, I'm considering taking up this issue, just have a couple clarification questions:

dblock commented 11 months ago
  • Would this issue require someone moving all of the type information from each .pyi stub file into it's corresponding Python file?

Probably. We don't want to lose type information.

Is there anything beyond that description of steps that would also needed to be done?

I don't have it. I tried to express "the problem" :)

Djcarrillo6 commented 11 months ago

Thanks @dblock! I'd like to request assignment on this issue.

Djcarrillo6 commented 11 months ago

Hi @dblock, I'm very sorry but can I be unassigned from this issue. I think I bit off more than I can chew with respect to the difficultly level of this fix. I'm going to pick another issue shortly that is more inline with my experience level with the repo.

dblock commented 11 months ago

Much of this change needs to be done in the client generator, so we should get https://github.com/opensearch-project/opensearch-py/pull/543 in first.

dblock commented 11 months ago

Current state: https://github.com/opensearch-project/opensearch-py/compare/main...dblock:merge-pyi?expand=1

$ nox -rs format

Found 354 errors in 51 files (checked 91 source files)
nox > Command mypy --strict opensearchpy/ failed with exit code 1
dblock commented 11 months ago

I finished this in https://github.com/opensearch-project/opensearch-py/pull/563. If anyone has any good reasons not to do it, speak up!