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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PEP 561 Compatiblity #41

Merged
merged 2 commits into from Mar 8, 2022
Merged

Add PEP 561 Compatiblity #41

merged 2 commits into from Mar 8, 2022

Conversation

harens
Copy link
Contributor

@harens harens commented Mar 8, 2022

Hi there 馃憢

I just saw #39 and thought it might be useful to add a py.typed file. This allows downstream projects to use the type hints for things like PyCharm, mypy, pytype etc.

Without this file, mypy currently complains in projects that use rich-click:

error: Skipping analyzing "rich_click": module is installed, but missing library stubs or py.typed marker

The py.typed file makes rich-click PEP-561 compatible, and allows the inline types to be used. See here for further explanation.

Allow typing info to be used downstream
@ewels
Copy link
Owner

ewels commented Mar 8, 2022

Nice! Yeah, @Jorricks has been helping with this as I am new to Python typing.. 馃懘馃徎

Should we be running mypy with GitHub Actions to check for errors in the typing?

@ewels ewels merged commit 6d444bf into ewels:main Mar 8, 2022
@Jorricks
Copy link
Contributor

Jorricks commented Mar 8, 2022

Oh that's a good pointer! I never created such a file.
Let's add it indeed.

Regarding running MyPy, that's included in the pre-commit. I'll continue working on that tomorrow.

@harens
Copy link
Contributor Author

harens commented Mar 8, 2022

Thank you @ewels for merging, and thanks @Jorricks for your effort on the typing front. 馃憤 I know from experience just how much work that can be, so I really appreciate what you're doing.

Should we be running mypy with GitHub Actions to check for errors in the typing?

Regarding running MyPy, that's included in the pre-commit.

Do you trust that the developer will always install the pre commit? ;)

As for mypy in general, I'd say it's quite a double-edged sword. It's saved me from mistakes in the past, but it takes a lot of effort to squash out errors from mypy, let alone mypy --strict. You might prefer the beartype approach of checking types at runtime:

From @leycec:

You want to write code rather than fight a static type checker, because static type inference of a dynamically-typed language is guaranteed to fail and frequently does. If you've ever cursed the sky after suffixing working code incorrectly typed by mypy with non-portable vendor-specific pragmas like # type: ignore[{unreadable_error}], beartype was written for you.

@ewels
Copy link
Owner

ewels commented Mar 8, 2022

Do you trust that the developer will always install the pre commit? ;)

The GitHub Actions CI workflow runs the pre-commit checks, so they don't need to - the PR checks will fail if it doesn't validate. See #40

@Jorricks
Copy link
Contributor

Jorricks commented Mar 8, 2022

Well in my opinion, MyPy is indeed a tiny bit annoying but it prevents so many bugs.
Furthermore, it makes sure you are set up for success around None values. And so much more.. I always felt it is worth the extra effort.

@@ -7,4 +7,5 @@
"rich>=10",
"importlib-metadata; python_version < '3.8'",
],
package_data={"rich-click": ["py.typed"]},
Copy link
Owner

@ewels ewels Mar 8, 2022

Choose a reason for hiding this comment

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

Bit late now, but maybe we could put this into setup.cfg instead? Most stuff is in there, it's only the requirements that are in setup.py so that they can be picked up by GitHub:

# Dependencies are in setup.py for GitHub's dependency graph.

I assume it would still work if in the .cfg file instead?

@harens harens deleted the py-typed branch March 29, 2022 08:54
Stealthii added a commit to Stealthii/rich-click that referenced this pull request Jun 8, 2022
1074477 in ewels#41 introduced the py.typed file, for marking the package as
containing typehints as part of PEP 561, however the package name is
incorrect and as such is not included in built wheels.
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.

None yet

3 participants