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

MYPY Types added #97

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

MYPY Types added #97

wants to merge 7 commits into from

Conversation

gagandeepp
Copy link
Contributor

@gagandeepp gagandeepp commented Oct 15, 2024

Brief Summary of Changes

Fixes: cloudinary/pycloudinary#377

What does this PR address?

  • GitHub issue (Add reference - #XX)
  • Refactoring
  • New feature
  • Bug fix
  • Adds more tests

Are tests included?

  • Yes
  • No

Reviewer, please note:

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I ran the full test suite before pushing the changes and all the tests pass.

Before :
image

After: Old screenshot currently on 18 errors are there
image

@gagandeepp
Copy link
Contributor Author

@const-cloudinary Does the initial changes looks good?

@gagandeepp gagandeepp changed the title Stubs added MYPY Types added Oct 15, 2024
@gagandeepp
Copy link
Contributor Author

@const-cloudinary Does the initial changes looks good?

Once you gives go ahead, I will implement the rest of the stubs and also add a precommit hook in the repository

@gagandeepp gagandeepp marked this pull request as draft October 16, 2024 02:23
@gagandeepp gagandeepp marked this pull request as ready for review October 16, 2024 02:24
@const-cloudinary
Copy link
Contributor

@gagandeepp , yeah looks great, please go ahead!

@const-cloudinary
Copy link
Contributor

@gagandeepp , this is what I see:

Screenshot 2024-10-16 at 19 44 17

@gagandeepp
Copy link
Contributor Author

@gagandeepp , this is what I see:

Screenshot 2024-10-16 at 19 44 17

Screenshot 2024-10-16 204200

@gagandeepp
Copy link
Contributor Author

@const-cloudinary Latest status:
image

@gagandeepp
Copy link
Contributor Author

@const-cloudinary Can you please take a look at this?

Copy link
Contributor

@const-cloudinary const-cloudinary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gagandeepp , Thank you for your PR!

This looks like a huge overhead of supporting such files, like, for every new option that is added, it needs to be added twice.

For example duplication of all @command definitions, FORMAT_ALIASES, not_callable.

Isn't it better to combine pyi and py files into one?

If no, is there any sort of automation? Like GitHub action? To make sure we don't forget to update pyi files ?

@gagandeepp
Copy link
Contributor Author

@gagandeepp , Thank you for your PR!

This looks like a huge overhead of supporting such files, like, for every new option that is added, it needs to be added twice.

For example duplication of all @command definitions, FORMAT_ALIASES, not_callable.

Isn't it better to combine pyi and py files into one?

If no, is there any sort of automation? Like GitHub action? To make sure we don't forget to update pyi files ?

Let me take a look of

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MyPy types
2 participants