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

[Feature] Add type hints and mypy support #306

Merged
merged 18 commits into from
Jan 6, 2024

Conversation

qequ
Copy link
Contributor

@qequ qequ commented Nov 11, 2023

Updated package so it's mypy compatible

Fixes #296

qequ added 10 commits November 9, 2023 21:54
Signed-off-by: Alvaro Frias Garay <[email protected]>
Signed-off-by: Alvaro Frias Garay <[email protected]>
Signed-off-by: Alvaro Frias Garay <[email protected]>
Signed-off-by: Alvaro Frias Garay <[email protected]>
Signed-off-by: Alvaro Frias Garay <[email protected]>
Signed-off-by: Alvaro Frias Garay <[email protected]>
Signed-off-by: Alvaro Frias Garay <[email protected]>
Signed-off-by: Alvaro Frias Garay <[email protected]>
Signed-off-by: Alvaro Frias Garay <[email protected]>
Signed-off-by: Alvaro Frias Garay <[email protected]>
@skorokithakis
Copy link
Collaborator

Hello, thanks for this! Did you use Black on the code? If so, could you please add it to CI?

Signed-off-by: Alvaro Frias Garay <[email protected]>
@qequ
Copy link
Contributor Author

qequ commented Nov 12, 2023

@skorokithakis I've added a CI job for check black linting 👍

qequ added 4 commits November 12, 2023 17:13
Signed-off-by: Alvaro Frias Garay <[email protected]>
Signed-off-by: Alvaro Frias Garay <[email protected]>
Signed-off-by: Alvaro Frias Garay <[email protected]>
black job

Signed-off-by: Alvaro Frias Garay <[email protected]>
@skorokithakis
Copy link
Collaborator

This looks good, thank you! To speed up the review, is there any way you could make your changes to schema.py and not run Black on it? Then I can review only your changes, and I'll run Black and push to your PR, and then merge.

@qequ
Copy link
Contributor Author

qequ commented Nov 12, 2023

@skorokithakis tough I ran Black there are no significant changes made on the code by Black. At most there are three or four methods/functions signatures formatted to multiple lines, but most of the code remains unchanged

@skorokithakis
Copy link
Collaborator

Oh, all those changes are manual? OK, I will review ASAP, thanks!

@qequ
Copy link
Contributor Author

qequ commented Dec 7, 2023

@skorokithakis any updates on this?

@skorokithakis
Copy link
Collaborator

Sorry, this looks good to me. Which Python versions can run this? I worry that it uses syntax that's a bit too recent...

@qequ
Copy link
Contributor Author

qequ commented Dec 18, 2023

@skorokithakis I have made some updates and tested it up to python3.6 and works normally

@skorokithakis
Copy link
Collaborator

skorokithakis commented Dec 18, 2023

When you say "up to", what do you mean? 2.7 to 3.6? Surely it can't be 2.7-compatible any more.

@qequ
Copy link
Contributor Author

qequ commented Dec 19, 2023

@skorokithakis I've tested from 3.6 to 3.10 and works fine. 3.5 and backwards versions doesn't support types

@skorokithakis
Copy link
Collaborator

In that case, that's fine, but the setup.py classifiers/etc needs to be changed to the proper supported versions.

@qequ
Copy link
Contributor Author

qequ commented Jan 6, 2024

@skorokithakis updated classifiers

Copy link
Collaborator

@skorokithakis skorokithakis left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@skorokithakis skorokithakis merged commit c25f41e into keleshev:master Jan 6, 2024
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 support
2 participants