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 documentation page for typing-extensions #166

Merged
merged 14 commits into from May 21, 2023

Conversation

JelleZijlstra
Copy link
Member

Currently, typing-extensions is documented only through the README and CHANGELOG. This is hard to navigate, so I feel it's time for a documentation page dedicated to typing-extensions.

We can host it at typing-extensions.readthedocs.io.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

This is great, and very much needed — thank you!

Some comments, mostly pretty minor:

doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

This is great, and very much needed — thank you!

Some comments, mostly pretty minor:

doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

A few more small points:

doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Outdated
Comment on lines 594 to 595
Certain types have incorrect runtime behavior due to limitations of older
versions of the typing module:
Copy link
Member

Choose a reason for hiding this comment

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

Make it slightly clearer that this is a distinct point to the previous paragraph:

Suggested change
Certain types have incorrect runtime behavior due to limitations of older
versions of the typing module:
As well as this, certain types have incorrect runtime behavior due to
limitations of older versions of the typing module:

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually think we should get rid of this catch-all section and document these limitations only under specific objects. I'll move a version of this text to the ParamSpec docs instead, and get rid of the sentence about Sequence/Reversible which I think is only relevant to 3.5-era changes.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that makes sense. Sounds like a plan.

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't figure out what's supposedly wrong with Concatenate, so I'm going to add a note to ParamSpec only.

Python 3.8.16 (default, Dec 26 2022, 20:36:56) 
[Clang 14.0.0 (clang-1400.0.29.202)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from typing_extensions import Concatenate, get_args, get_origin, ParamSpec
>>> P = ParamSpec("P")
>>> c = Concatenate[int, P]
>>> get_origin(c)
typing_extensions.Concatenate
>>> get_args(c)
(<class 'int'>, ~P)

Copy link
Member

Choose a reason for hiding this comment

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

I couldn't figure out what's supposedly wrong with Concatenate, so I'm going to add a note to ParamSpec only.

Makes sense, though #110 might be worth a mention in the Concatenate entry. I took a brief look at it a while back, and it looks... challenging to backport.

Copy link
Member

Choose a reason for hiding this comment

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

#48 as well...

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, added those. I also noticed that the :gh: links to CPython weren't working, so I added a custom Sphinx extension to make them work (possibly overkill?).

Copy link
Member

Choose a reason for hiding this comment

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

possibly overkill?

Definitely a Choice, but I'll support you in it

JelleZijlstra and others added 6 commits May 21, 2023 13:49
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
- The stuff about Reversible is no longer relevant
- Can't confirm issues with Concatenate
- Add note specifically to ParamSpec docs
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Looks great!

@JelleZijlstra JelleZijlstra merged commit 024d465 into python:main May 21, 2023
16 checks passed
@JelleZijlstra JelleZijlstra deleted the doc branch May 21, 2023 22:41
@JelleZijlstra
Copy link
Member Author

Next step: get this deployed. Other followups:

  • Migrate changelog to RST and put it on this docs page
  • Trim down the README to remove redundancy with this page
  • Consider extending the introduction, maybe a section specifically on the versioning scheme and/or backward compatibility

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

2 participants