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

Fix type annotations with using type.List. #90

Merged
merged 2 commits into from
Apr 29, 2024
Merged

Conversation

azriel1rf
Copy link
Collaborator

Fix type annotations in Python for issue #75
This pull request addresses the type annotation errors raised by mypy when running mypy .. The necessary changes have been made to resolve these errors.

Additonal Notes:

@azriel1rf azriel1rf requested a review from HenryRLee April 29, 2024 05:31
Copy link
Owner

@HenryRLee HenryRLee left a comment

Choose a reason for hiding this comment

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

Hey @azriel1rf, nice to see you back!

The PR looks good to me. Ready to merge.

My question is, does this change break our library in Python3.7? If it does, we may have to increase the major version number (i.e. 0.6.0) when we release a new package. Also, maybe it is a good time to start using different versioning in our Python and C++ library. (Since C++ has already been in 0.6.0 with new PLO5 and PLO6 features, and some features deprecated.) But these are the next steps.

@azriel1rf
Copy link
Collaborator Author

To clarify, this change doesn't affect Python 3.7 functionality, as the type annotation issue is specific to versions below 3.9. However, I removed Python 3.7 support since it has reached end-of-life.

Regarding versioning, while incrementing for language-specific features may seem frequent, we should consider the impact on users.

How about creating a development branch as a staging area to merge and test changes before releasing to the master branch? This will ensure a more controlled release cycle and consistent versioning.
What do you think about this approach? Let me know if you have any further ideas or concerns.

@HenryRLee
Copy link
Owner

HenryRLee commented Apr 29, 2024

Hi @azriel1rf, I think your approach is good. I didn't choose this approach previously because I found it overfitting for this small repo. However, since now we have multiple languages and increasing number of bug fixes and features, it is time for us to change the style of managing the repo now.

The only catch is that we need to make sure contributors are checking out the code in development branch. I'll add a note in the CONTRIBUTING.md file.

Let me merge this PR, and then I'll create a development branch named develop.

@HenryRLee HenryRLee merged commit bf1a1dd into master Apr 29, 2024
6 checks passed
@azriel1rf
Copy link
Collaborator Author

@HenryRLee Thanks!

@azriel1rf
Copy link
Collaborator Author

@HenryRLee Thank you for creating the develop branch. I appreciate your support in implementing this new development workflow.
One important thing to consider is updating our CI configuration in ci.yml. We need to ensure that the CI pipeline is triggered for pushes and pull requests to both the master and develop branches.

@azriel1rf azriel1rf deleted the fix_type_annotations branch April 29, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants