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 content from typeshed/CONTRIBUTING.md #1882

Merged
merged 15 commits into from
Dec 17, 2024
Merged

Conversation

yangdanny97
Copy link
Contributor

@yangdanny97 yangdanny97 commented Nov 7, 2024

This PR merges non-typeshed-specific content on writing stubs into main typing guide.

https://github.com/python/typeshed/blob/main/CONTRIBUTING.md

This would let us reference this guide from typeshed's CONTRIBUTING.md and delete the duplicate sections.

#1880

@@ -113,18 +118,6 @@ Stub Content
This section documents best practices on what elements to include or
leave out of stub files.

Modules excluded fom stubs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved this down cuz it felt weird for the "stub content" section to start with "what you shouldn't include"

@yangdanny97 yangdanny97 marked this pull request as ready for review November 13, 2024 22:17
@JelleZijlstra JelleZijlstra self-requested a review November 15, 2024 14:22
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
Avoid invariant collection types (`list`, `dict`) for function parameters,
in favor of covariant types like `Mapping` or `Sequence`.

Avoid union return types. See https://github.com/python/mypy/issues/1693
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably for a future PR, but it would be nice if the content of python/mypy#1693 and python/mypy#7214 below could be copied into the guides. Having pieces of general typing advice hidden away in mypy issues feels sort of odd.

docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@rchen152 rchen152 left a comment

Choose a reason for hiding this comment

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

Looks good to me aside from a couple minor comments.

@JelleZijlstra Do you want to review this as well?

docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved

Keep the deprecation message concise, but try to mention the projected
version when the functionality is to be removed, and a suggested
replacement.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we say that if a deprecated feature is still very widely used and was deprecated only recently, it may be best to wait before adding @deprecated?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so. If the feature is still in wide use, that's an argument against deprecating the feature, but if the runtime code decides to deprecate something, the stubs should communicate that fact.

docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
docs/guides/writing_stubs.rst Outdated Show resolved Hide resolved
@rchen152
Copy link
Collaborator

Looks like all comments have been addressed. I did one more style/formatting pass. Thanks @yangdanny97!

@rchen152 rchen152 merged commit a434c99 into python:main Dec 17, 2024
4 checks passed
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.

5 participants