gh-128335: Make slice generic at runtime #128336
Conversation
Gobot1234
commented
Dec 29, 2024
•
- Issue: Make slice generic #128335
| @@ -560,6 +560,7 @@ static PyMethodDef slice_methods[] = { | |||
| METH_O, slice_indices_doc}, | |||
| {"__reduce__", (PyCFunction)slice_reduce, | |||
| METH_NOARGS, reduce_doc}, | |||
| {"__class_getitem__", Py_GenericAlias, METH_O|METH_CLASS, "See PEP 585"}, | |||
There was a problem hiding this comment.
This docstring seems misleading, as PEP 585 doesn't mention the slice class.
(Maybe PEP 585 could refer to https://typing.readthedocs.io/en/latest/guides/modernizing.html#generics-in-the-typing-module? Right now it links to documentation for __class_getitem__ that on its own isn't a replacement for PEP 585 from a reader's perspective)
There was a problem hiding this comment.
Agreed it is maybe a little silly to be putting PEPs in these things when they aren't active docs.
There was a problem hiding this comment.
PRs to the PEPs repo improving things like this are welcome!
There was a problem hiding this comment.
Curious if @AlexWaygood / @JelleZijlstra have opinions on this. This is done in other places. I seem to have time travelled here
There was a problem hiding this comment.
I do agree that our docstrings probably also shouldn't be referencing PEPs, which are, as you both note, historical documents recording decisions that were made at one point in time rather than living documentation. But all the other __class_getitem__ docstrings say this currently, so I think it makes sense to follow suit here and change all the __class_getitem__ docstrings to something better all at once as a followup
Misc/NEWS.d/next/Core_and_Builtins/2024-12-29-21-33-08.gh-issue-128334.3c5Nou.rst
Outdated
Show resolved
Hide resolved
…e-128334.3c5Nou.rst Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
|
I think this calls for a whatsnew entry. Something like * The :class:`slice` type now supports subscription,
making it a :term:`generic type`.
(Contributed by James Hilton-Balfe in :gh:`128335`.)We should probably also add a |