search

jquast / wcwidth

Python library that measures the width of unicode strings rendered to a terminal
Other
391 stars 58 forks source link

add typing and py.typed #71

Open trim21 opened 1 year ago

trim21 commented 1 year ago

consider we still support python2.7, this PR add type stub instead of inline types.

trim21 commented 1 year ago

@jquast

GalaxySnail commented 1 year ago

Sorry to keep you waiting. I'm currently working on running tests using GitHub Actions, and the next step will be integrating mypy.

trim21 commented 1 year ago

Sorry to keep you waiting. I'm currently working on running tests using GitHub Actions, and the next step will be integrating mypy.

I can help with that

trim21 commented 11 months ago

@GalaxySnail @jquast

codecov[bot] commented 11 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (4f5ba62) 100.00% compared to head (4061076) 100.00%.

:exclamation: Current head 4061076 differs from pull request most recent head 3190837. Consider uploading reports for the commit 3190837 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #71 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 5 5 Lines 105 105 Branches 25 25 ========================================= Hits 105 105 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

trim21 commented 11 months ago

@GalaxySnail @jquast can this PR get some code review？

GalaxySnail commented 11 months ago

I'm still trying to figure out what the best practice is to test typing stubs, especially back in Python 2.7. Do you have any ideas?

trim21 commented 11 months ago

I'm still trying to figure out what the best practice is to test typing stubs, especially back in Python 2.7. Do you have any ideas?

run mypy stubtest to check if type stub is in-sync with source code I guess? Without inline type annotations there is no need to run mypy I think.

https://mypy.readthedocs.io/en/stable/stubtest.html

No need to run stubtest with runtime 2.7, you can run it in latest python version. type stub are expected to be parsed as latest python version.

trim21 commented 11 months ago

I'm still trying to figure out what the best practice is to test typing stubs, especially back in Python 2.7. Do you have any ideas?

just add ci job for type stubs testing

trim21 commented 11 months ago

@GalaxySnail any review?

jquast commented 11 months ago

Looks OK to me... @GalaxySnail ?

trim21 commented 11 months ago

CI is broken

GalaxySnail commented 11 months ago

CI is broken

Yes. (That's what tests intend to do.) We should fix them before merging.

trim21 commented 11 months ago

CI is broken

Yes. (That's what tests intend to do.) We should fix them before merging.

no... As I mentioned before, you only need to run mypy's stubtest in latest python, no need to run it in old python version.

GalaxySnail commented 11 months ago

no... As I mentioned before, you only need to run mypy's stubtest in latest python, no need to run it in old python version.

I doubt it. If someone is using wcwidth on an older python version, they will fail just like what happenned in CI, which makes typing stubs useless.

trim21 commented 11 months ago

no... As I mentioned before, you only need to run mypy's stubtest in latest python, no need to run it in old python version.

I doubt it. If someone is using wcwidth on an older python version, they will fail just like what happenned in CI, which makes typing stubs useless.

No, because type stub (pyi) is intend to be parsed by tools like mypy or pyright(another type checking tool), python runtime doesn't execute it. This is also the reason why a package support python2.7 can work with type stub. So you only need to check it in latest python version.

and ci failed because you try to run mypy in old version of python, mypy need python>=3.6. it has nothing to do with type stub.

GalaxySnail commented 11 months ago

and ci failed because you try to run mypy in old version of python, mypy need python>=3.6. it has nothing to do with type stub.

OK, thanks for your clarification. So we should run mypy on python>=3.6 only. AFAIK the latest version of mypy which supports python2 is mypy 0.971 [1].

[1] https://mypy-lang.blogspot.com/2022/07/mypy-0971-released.html

trim21 commented 11 months ago

and ci failed because you try to run mypy in old version of python, mypy need python>=3.6. it has nothing to do with type stub.

OK, thanks for your clarification. So we should run mypy on python>=3.6 only. AFAIK the latest version of mypy which supports python2 is mypy 0.971 [1].

[1] https://mypy-lang.blogspot.com/2022/07/mypy-0971-released.html

we don't need to use this version， only downstream developers who are still using Python2 need to use this version， we can run mypy in python 3.12 and use latest mypy version.

GalaxySnail commented 11 months ago

OK, thanks for your clarification. So we should run mypy on python>=3.6 only. AFAIK the latest version of mypy which supports python2 is mypy 0.971 [1]. [1] https://mypy-lang.blogspot.com/2022/07/mypy-0971-released.html

we don't need to use this version， only downstream developers who are still using Python2 need to use this version， we can run mypy in python 3.12 and use latest mypy version.

No, if we want downstream users to use it, we should test it, otherwise it will probably break for downstream users.

trim21 commented 11 months ago

OK, thanks for your clarification. So we should run mypy on python>=3.6 only. AFAIK the latest version of mypy which supports python2 is mypy 0.971 [1]. [1] https://mypy-lang.blogspot.com/2022/07/mypy-0971-released.html

we don't need to use this version， only downstream developers who are still using Python2 need to use this version， we can run mypy in python 3.12 and use latest mypy version.

No, if we want downstream users to use it, we should test it, otherwise it will probably break for downstream users.

For python code, yes, I agree with you. But typing stub is another scope.

trim21 commented 11 months ago

OK, thanks for your clarification. So we should run mypy on python>=3.6 only. AFAIK the latest version of mypy which supports python2 is mypy 0.971 [1]. [1] https://mypy-lang.blogspot.com/2022/07/mypy-0971-released.html

we don't need to use this version， only downstream developers who are still using Python2 need to use this version， we can run mypy in python 3.12 and use latest mypy version.

No, if we want downstream users to use it, we should test it, otherwise it will probably break for downstream users.

For python code, yes, I agree with you. But typing stub is another scope, they are not actually code to be executed.

trim21 commented 11 months ago

Sorry, the test you added failed ci's python2.7 testing job, I don't know how to fix it.

GalaxySnail commented 11 months ago

Sorry, the test you added failed ci's python2.7 testing job, I don't know how to fix it.

That's OK, thanks for your efforts.

Another idea, you can submit a stub-only package for wcwidth on typeshed, so that users who need it can easily install it by pip install types-wcwidth, and users who don't need it will not be affected.

jquast commented 11 months ago

Thank you both all the attention and efforts and concerns. I understand that python 2 compatibility is difficult, especially for typing.

I have created Issue #102 with my thoughts about dropping support. I think restricting future releases to python 3.6+ would allow easier typing in-file, without stubs and make this easier for everyone?

trim21 commented 11 months ago

Thank you both all the attention and efforts and concerns. I understand that python 2 compatibility is difficult, especially for typing.

I have created Issue #102 with my thoughts about dropping support. I think restricting future releases to python 3.6+ would allow easier typing in-file, without stubs and make this easier for everyone?

I can wait until then