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

add typing and py.typed #71

Closed
wants to merge 23 commits into from
Closed

add typing and py.typed #71

wants to merge 23 commits into from

Conversation

trim21
Copy link
Contributor

@trim21 trim21 commented Jul 6, 2023

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

@trim21
Copy link
Contributor Author

trim21 commented Sep 12, 2023

@jquast

@GalaxySnail
Copy link
Collaborator

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

@trim21
Copy link
Contributor Author

trim21 commented Sep 12, 2023

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
Copy link
Contributor Author

trim21 commented Nov 17, 2023

@GalaxySnail @jquast

Copy link

codecov bot commented Nov 17, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

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

❗ 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
@@            Coverage Diff            @@
##            master       #71   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          105       105           
  Branches        25        25           
=========================================
  Hits           105       105           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@trim21
Copy link
Contributor Author

trim21 commented Nov 20, 2023

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

@GalaxySnail
Copy link
Collaborator

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
Copy link
Contributor Author

trim21 commented Nov 20, 2023

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
Copy link
Contributor Author

trim21 commented Nov 20, 2023

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
Copy link
Contributor Author

trim21 commented Nov 20, 2023

@GalaxySnail any review?

@jquast
Copy link
Owner

jquast commented Nov 21, 2023

Looks OK to me... @GalaxySnail ?

jquast
jquast previously approved these changes Nov 21, 2023
@trim21
Copy link
Contributor Author

trim21 commented Nov 21, 2023

CI is broken

@trim21
Copy link
Contributor Author

trim21 commented Nov 21, 2023

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
Copy link
Collaborator

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
Copy link
Contributor Author

trim21 commented Nov 21, 2023

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
Copy link
Contributor Author

trim21 commented Nov 21, 2023

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
Copy link
Contributor Author

trim21 commented Nov 21, 2023

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

@GalaxySnail
Copy link
Collaborator

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
Copy link
Owner

jquast commented Nov 21, 2023

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
Copy link
Contributor Author

trim21 commented Nov 22, 2023

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

Jayman2000 added a commit to Jayman2000/jasons-pre-commit-hooks that referenced this pull request Jan 16, 2024
Before this commit, there was repetition in the PRE_COMMIT_REPOS_BY_PATH
dictionary:

    PRE_COMMIT_REPOS_BY_PATH: Final = (
        ...,
        ('**.yaml', PCR_YAMLLINT),
        ('**.yml', PCR_YAMLLINT),
        ...
    )

That repetition was necessary. Before this change, I had to repeat
myself in order to specify that both .yaml files and .yml files should
enable PCR_YAMLLINT.

I want to be able to specify multiple globs to make it easier for me to
create two future commits: a type-stub-related commit and a
wcwidth-related commit. The wcwidth-related commit will add code that
depends on wcwidth [1]. Unfortunately, wcwidth doesn’t have any type
hints yet [2], and there doesn’t seem to be a PEP 561 [3] stub package
for wcwidth. When a module isn’t typed and there’s no stub package for
that module, mypy’s recommendation is to write your own stub files [4].

Before that future wcwidth-related commit, I’m going to add a
type-stub-related commit. That type-stub-related commit will make it so
that all of the pre-commit hooks for .py files also run for .pyi files
[5].

The main motivation behind this commit is to make it easier for me to
write that future type-stub-related commit.

[1]: <https://pypi.org/project/wcwidth/>
[2]: <jquast/wcwidth#71>
[3]: <https://peps.python.org/pep-0561/>
[4]: <https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-library-stubs-or-py-typed-marker>
[5]: <https://mypy.readthedocs.io/en/stable/stubs.html>

TODO:
• Make sure that links are backed up in the Internet Archive
• Add the type-stub-related-commit after this one.
Jayman2000 added a commit to Jayman2000/jasons-pre-commit-hooks that referenced this pull request Jan 17, 2024
Before this commit, there was repetition in the PRE_COMMIT_REPOS_BY_PATH
dictionary:

    PRE_COMMIT_REPOS_BY_PATH: Final = (
        ...,
        ('**.yaml', PCR_YAMLLINT),
        ('**.yml', PCR_YAMLLINT),
        ...
    )

That repetition was necessary. Before this change, I had to repeat
myself in order to specify that both .yaml files and .yml files should
enable PCR_YAMLLINT.

I want to be able to specify multiple globs to make it easier for me to
create two future commits: a type-stub-related commit and a
wcwidth-related commit. The wcwidth-related commit will add code that
depends on wcwidth [1]. Unfortunately, wcwidth doesn’t have any type
hints yet [2], and there doesn’t seem to be a PEP 561 [3] stub package
for wcwidth. When a module isn’t typed and there’s no stub package for
that module, mypy’s recommendation is to write your own stub files [4].

Before that future wcwidth-related commit, I’m going to add a
type-stub-related commit. That type-stub-related commit will make it so
that all of the pre-commit hooks for .py files also run for .pyi files
[5].

The main motivation behind this commit is to make it easier for me to
write that future type-stub-related commit.

[1]: <https://pypi.org/project/wcwidth/>
[2]: <jquast/wcwidth#71>
[3]: <https://peps.python.org/pep-0561/>
[4]: <https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-library-stubs-or-py-typed-marker>
[5]: <https://mypy.readthedocs.io/en/stable/stubs.html>

TODO:
• Finish the commits that come after this one.
• Make sure that links are backed up in the Internet Archive
• Once the type-stub-related-commit and the wcwidth-related-commit are
done, add a Git note to this commit that references those commits.
Jayman2000 added a commit to Jayman2000/jasons-pre-commit-hooks that referenced this pull request Jan 17, 2024
This will make sure that Python-related hints for contributors and
pre-commit hooks even if repos only contain stub files [1]. The main
motivation behind creating this commit is to prepare for a future commit
that will add Python stub files to this repo.

That future commit will add wcwidth [2] as a dependency. Unfortunately,
wcwidth doesn’t have any type hints yet [3], and there doesn’t seem to
be a PEP 561 [4] stub package for wcwidth. When a module isn’t typed and
there’s no stub package for that module, mypy’s recommendation is to
write your own stub files [5]. As a result, that future commit is going
to add .pyi files to this repo.

Since that future commit is going to add a new type of file to this
repo, I thought that it would make sense to preemptively add support for
that file type to repo-style-checker.

[1]: <https://mypy.readthedocs.io/en/stable/stubs.html>
[2]: <https://pypi.org/project/wcwidth/>
[3]: <jquast/wcwidth#71>
[4]: <https://peps.python.org/pep-0561/>
[5]: <https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-library-stubs-or-py-typed-marker>

TODO:
• Test
• Finish the commit that comes after this one.
• Add the future wcwidth-related commit.
• Add a Git note to this commit with a reference to the future commit
that adds type stubs.
Jayman2000 added a commit to Jayman2000/jasons-pre-commit-hooks that referenced this pull request Jan 17, 2024
This will make sure that Python-related hints for contributors and
pre-commit hooks even if repos only contain stub files [1]. The main
motivation behind creating this commit is to prepare for a future commit
that will add Python stub files to this repo.

That future commit will add wcwidth [2] as a dependency. Unfortunately,
wcwidth doesn’t have any type hints yet [3], and there doesn’t seem to
be a PEP 561 [4] stub package for wcwidth. When a module isn’t typed and
there’s no stub package for that module, mypy’s recommendation is to
write your own stub files [5]. As a result, that future commit is going
to add .pyi files to this repo.

Since that future commit is going to add a new type of file to this
repo, I thought that it would make sense to preemptively add support for
that file type to repo-style-checker.

[1]: <https://mypy.readthedocs.io/en/stable/stubs.html>
[2]: <https://pypi.org/project/wcwidth/>
[3]: <jquast/wcwidth#71>
[4]: <https://peps.python.org/pep-0561/>
[5]: <https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-library-stubs-or-py-typed-marker>

TODO:
• Finish the commit that comes after this one.
• Add the future wcwidth-related commit.
• Add a Git note to this commit with a reference to the future commit
that adds type stubs.
Jayman2000 added a commit to Jayman2000/jasons-pre-commit-hooks that referenced this pull request Jan 17, 2024
Before this commit, there was repetition in the PRE_COMMIT_REPOS_BY_PATH
dictionary:

    PRE_COMMIT_REPOS_BY_PATH: Final = (
        ...,
        ('**.yaml', PCR_YAMLLINT),
        ('**.yml', PCR_YAMLLINT),
        ...
    )

That repetition was necessary. Before this change, I had to repeat
myself in order to specify that both .yaml files and .yml files should
enable PCR_YAMLLINT.

I want to be able to specify multiple globs to make it easier for me to
create two future commits: a type-stub-related commit and a
wcwidth-related commit. The wcwidth-related commit will add code that
depends on wcwidth [1]. Unfortunately, wcwidth doesn’t have any type
hints yet [2], and there doesn’t seem to be a PEP 561 [3] stub package
for wcwidth. When a module isn’t typed and there’s no stub package for
that module, mypy’s recommendation is to write your own stub files [4].

Before that future wcwidth-related commit, I’m going to add a
type-stub-related commit. That type-stub-related commit will make it so
that all of the pre-commit hooks for .py files also run for .pyi files
[5].

The main motivation behind this commit is to make it easier for me to
write that future type-stub-related commit.

[1]: <https://pypi.org/project/wcwidth/>
[2]: <jquast/wcwidth#71>
[3]: <https://peps.python.org/pep-0561/>
[4]: <https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-library-stubs-or-py-typed-marker>
[5]: <https://mypy.readthedocs.io/en/stable/stubs.html>

TODO:
• Finish the commits that come after this one.
• Make sure that links are backed up in the Internet Archive
Jayman2000 added a commit to Jayman2000/jasons-pre-commit-hooks that referenced this pull request Jan 17, 2024
This will make sure that Python-related hints for contributors and
pre-commit hooks even if repos only contain stub files [1]. The main
motivation behind creating this commit is to prepare for a future commit
that will add Python stub files to this repo.

That future commit will add wcwidth [2] as a dependency. Unfortunately,
wcwidth doesn’t have any type hints yet [3], and there doesn’t seem to
be a PEP 561 [4] stub package for wcwidth. When a module isn’t typed and
there’s no stub package for that module, mypy’s recommendation is to
write your own stub files [5]. As a result, that future commit is going
to add .pyi files to this repo.

Since that future commit is going to add a new type of file to this
repo, I thought that it would make sense to preemptively add support for
that file type to repo-style-checker.

[1]: <https://mypy.readthedocs.io/en/stable/stubs.html>
[2]: <https://pypi.org/project/wcwidth/>
[3]: <jquast/wcwidth#71>
[4]: <https://peps.python.org/pep-0561/>
[5]: <https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-library-stubs-or-py-typed-marker>

TODO:
• Finish the commit that comes after this one.
• Add the future wcwidth-related commit.
Jayman2000 added a commit to Jayman2000/jasons-pre-commit-hooks that referenced this pull request Jan 25, 2024
Before this commit, there was repetition in the PRE_COMMIT_REPOS_BY_PATH
dictionary:

    PRE_COMMIT_REPOS_BY_PATH: Final = (
        ...,
        ('**.yaml', PCR_YAMLLINT),
        ('**.yml', PCR_YAMLLINT),
        ...
    )

That repetition was necessary. Before this change, I had to repeat
myself in order to specify that both .yaml files and .yml files should
enable PCR_YAMLLINT.

I want to be able to specify multiple globs to make it easier for me to
create two future commits: a type-stub-related commit and a
wcwidth-related commit. The wcwidth-related commit will add code that
depends on wcwidth [1]. Unfortunately, wcwidth doesn’t have any type
hints yet [2], and there doesn’t seem to be a PEP 561 [3] stub package
for wcwidth. When a module isn’t typed and there’s no stub package for
that module, mypy’s recommendation is to write your own stub files [4].

Before that future wcwidth-related commit, I’m going to add a
type-stub-related commit. That type-stub-related commit will make it so
that all of the pre-commit hooks for .py files also run for .pyi files
[5].

The main motivation behind this commit is to make it easier for me to
write that future type-stub-related commit.

[1]: <https://pypi.org/project/wcwidth/>
[2]: <jquast/wcwidth#71>
[3]: <https://peps.python.org/pep-0561/>
[4]: <https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-library-stubs-or-py-typed-marker>
[5]: <https://mypy.readthedocs.io/en/stable/stubs.html>

TODO:
• Finish the commits that come after this one.
• Make sure that links are backed up in the Internet Archive
Jayman2000 added a commit to Jayman2000/jasons-pre-commit-hooks that referenced this pull request Jan 25, 2024
This will make sure that Python-related hints for contributors and
pre-commit hooks even if repos only contain stub files [1]. The main
motivation behind creating this commit is to prepare for a future commit
that will add Python stub files to this repo.

That future commit will add wcwidth [2] as a dependency. Unfortunately,
wcwidth doesn’t have any type hints yet [3], and there doesn’t seem to
be a PEP 561 [4] stub package for wcwidth. When a module isn’t typed and
there’s no stub package for that module, mypy’s recommendation is to
write your own stub files [5]. As a result, that future commit is going
to add .pyi files to this repo.

Since that future commit is going to add a new type of file to this
repo, I thought that it would make sense to preemptively add support for
that file type to repo-style-checker.

[1]: <https://mypy.readthedocs.io/en/stable/stubs.html>
[2]: <https://pypi.org/project/wcwidth/>
[3]: <jquast/wcwidth#71>
[4]: <https://peps.python.org/pep-0561/>
[5]: <https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-library-stubs-or-py-typed-marker>

TODO:
• Finish the commit that comes after this one.
• Add the future wcwidth-related commit.
Jayman2000 added a commit to Jayman2000/jasons-pre-commit-hooks that referenced this pull request Jan 25, 2024
Before this commit, there was repetition in the PRE_COMMIT_REPOS_BY_PATH
dictionary:

    PRE_COMMIT_REPOS_BY_PATH: Final = (
        ...,
        ('**.yaml', PCR_YAMLLINT),
        ('**.yml', PCR_YAMLLINT),
        ...
    )

That repetition was necessary. Before this change, I had to repeat
myself in order to specify that both .yaml files and .yml files should
enable PCR_YAMLLINT.

I want to be able to specify multiple globs to make it easier for me to
create two future commits: a type-stub-related commit and a
wcwidth-related commit. The wcwidth-related commit will add code that
depends on wcwidth [1]. Unfortunately, wcwidth doesn’t have any type
hints yet [2], and there doesn’t seem to be a PEP 561 [3] stub package
for wcwidth. When a module isn’t typed and there’s no stub package for
that module, mypy’s recommendation is to write your own stub files [4].

Before that future wcwidth-related commit, I’m going to add a
type-stub-related commit. That type-stub-related commit will make it so
that all of the pre-commit hooks for .py files also run for .pyi files
[5].

The main motivation behind this commit is to make it easier for me to
write that future type-stub-related commit.

[1]: <https://pypi.org/project/wcwidth/>
[2]: <jquast/wcwidth#71>
[3]: <https://peps.python.org/pep-0561/>
[4]: <https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-library-stubs-or-py-typed-marker>
[5]: <https://mypy.readthedocs.io/en/stable/stubs.html>

TODO:
• Make sure that links are backed up in the Internet Archive
Jayman2000 added a commit to Jayman2000/jasons-pre-commit-hooks that referenced this pull request Jan 25, 2024
This will make sure that Python-related hints for contributors and
pre-commit hooks even if repos only contain stub files [1]. The main
motivation behind creating this commit is to prepare for a future commit
that will add Python stub files to this repo.

That future commit will add wcwidth [2] as a dependency. Unfortunately,
wcwidth doesn’t have any type hints yet [3], and there doesn’t seem to
be a PEP 561 [4] stub package for wcwidth. When a module isn’t typed and
there’s no stub package for that module, mypy’s recommendation is to
write your own stub files [5]. As a result, that future commit is going
to add .pyi files to this repo.

Since that future commit is going to add a new type of file to this
repo, I thought that it would make sense to preemptively add support for
that file type to repo-style-checker.

[1]: <https://mypy.readthedocs.io/en/stable/stubs.html>
[2]: <https://pypi.org/project/wcwidth/>
[3]: <jquast/wcwidth#71>
[4]: <https://peps.python.org/pep-0561/>
[5]: <https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-library-stubs-or-py-typed-marker>

TODO:
• Add the future wcwidth-related commit.
Jayman2000 added a commit to Jayman2000/jasons-pre-commit-hooks that referenced this pull request Jan 25, 2024
Before this commit, there was repetition in the PRE_COMMIT_REPOS_BY_PATH
dictionary:

    PRE_COMMIT_REPOS_BY_PATH: Final = (
        ...,
        ('**.yaml', PCR_YAMLLINT),
        ('**.yml', PCR_YAMLLINT),
        ...
    )

That repetition was necessary. Before this change, I had to repeat
myself in order to specify that both .yaml files and .yml files should
enable PCR_YAMLLINT.

I want to be able to specify multiple globs to make it easier for me to
create two future commits: a type-stub-related commit and a
wcwidth-related commit. The wcwidth-related commit will add code that
depends on wcwidth [1]. Unfortunately, wcwidth doesn’t have any type
hints yet [2], and there doesn’t seem to be a PEP 561 [3] stub package
for wcwidth. When a module isn’t typed and there’s no stub package for
that module, mypy’s recommendation is to write your own stub files [4].

Before that future wcwidth-related commit, I’m going to add a
type-stub-related commit. That type-stub-related commit will make it so
that all of the pre-commit hooks for .py files also run for .pyi files
[5].

The main motivation behind this commit is to make it easier for me to
write that future type-stub-related commit.

[1]: <https://pypi.org/project/wcwidth/>
[2]: <jquast/wcwidth#71>
[3]: <https://peps.python.org/pep-0561/>
[4]: <https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-library-stubs-or-py-typed-marker>
[5]: <https://mypy.readthedocs.io/en/stable/stubs.html>
Jayman2000 added a commit to Jayman2000/jasons-pre-commit-hooks that referenced this pull request Jan 25, 2024
This will make sure that Python-related hints for contributors and
pre-commit hooks even if repos only contain stub files [1]. The main
motivation behind creating this commit is to prepare for a future commit
that will add Python stub files to this repo.

That future commit will add wcwidth [2] as a dependency. Unfortunately,
wcwidth doesn’t have any type hints yet [3], and there doesn’t seem to
be a PEP 561 [4] stub package for wcwidth. When a module isn’t typed and
there’s no stub package for that module, mypy’s recommendation is to
write your own stub files [5]. As a result, that future commit is going
to add .pyi files to this repo.

Since that future commit is going to add a new type of file to this
repo, I thought that it would make sense to preemptively add support for
that file type to repo-style-checker.

[1]: <https://mypy.readthedocs.io/en/stable/stubs.html>
[2]: <https://pypi.org/project/wcwidth/>
[3]: <jquast/wcwidth#71>
[4]: <https://peps.python.org/pep-0561/>
[5]: <https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-library-stubs-or-py-typed-marker>
@trim21 trim21 closed this Nov 27, 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.

3 participants