Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PROPOSAL] Merge .pyi types inline #536

Closed
dblock opened this issue Oct 12, 2023 · 7 comments · Fixed by #563
Closed

[PROPOSAL] Merge .pyi types inline #536

dblock opened this issue Oct 12, 2023 · 7 comments · Fixed by #563
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@dblock
Copy link
Member

dblock commented Oct 12, 2023

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.

@dblock dblock added enhancement New feature or request good first issue Good for newcomers labels Oct 12, 2023
@github-actions github-actions bot added the untriaged Need triage label Oct 12, 2023
@Djcarrillo6
Copy link
Contributor

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

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

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

@dblock
Copy link
Member Author

dblock commented Oct 13, 2023

  • 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
Copy link
Contributor

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

@saimedhi saimedhi removed the untriaged Need triage label Oct 13, 2023
@Djcarrillo6
Copy link
Contributor

Djcarrillo6 commented Oct 19, 2023

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
Copy link
Member Author

dblock commented Oct 24, 2023

Much of this change needs to be done in the client generator, so we should get #543 in first.

@dblock dblock self-assigned this Oct 24, 2023
@dblock
Copy link
Member Author

dblock commented Oct 25, 2023

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
Copy link
Member Author

dblock commented Nov 1, 2023

I finished this in #563. If anyone has any good reasons not to do it, speak up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants