Skip to content

remove python2, annotate #29

New issue

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

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

Already on GitHub? # to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

a-detiste
Copy link
Contributor

this is a draft, open to disscussion.

work can be split into two PR for an easier review:

  • remove Python2 support (first)
  • add annotations

Copy link
Owner

@fmenabe fmenabe left a comment

Choose a reason for hiding this comment

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

Hi, I review the PR. I have not much to say about type hints, I don't use them (yet) personally. I review hints and they seem coherents but I don't even know how to ensure they are correct (I used mypy on the file but not sure that's enough).

I also made comments about the addition of some variables instead of keeping the variables reassignments. Just curious is there a reason for this?

@a-detiste a-detiste reopened this Nov 2, 2023
@a-detiste
Copy link
Contributor Author

Please merge as-is ... I don't have much time anymore for annotations. But it is an incremental work anyway.

Greetings

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

Successfully merging this pull request may close these issues.

2 participants