-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
gh-67748: DOC:Add summary table for str methods in stdtypes.rst #1709
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t have much of an opinion about adding this table comparing the methods that are available. If others think it would be good to add, don’t let me get in your way :)
FWIW I don’t think it helps with Ezio’s original goal to remove duplication and make the documentation shorter. But I don’t see a good way to accomplish that and at the same time satisfy @rhettinger by keeping an entry for each method in alphabetical order.
Text and Binary Sequence Type Methods Summary | ||
============================================= | ||
The following table summarizes the text and binary sequence types methods by | ||
category. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this paragraph necessary? It seems obvious just given the section and table headings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, Martin. You're right. I wasn't quite sure where to place this overall and thought there would probably be suggestions, so I kept it vague. There wasn't a section that combined both text and binary sequence types before and I didn't know if people would like having str and bytes (and bytearray) types all in one table or if there should be a separate table for each one under the existing sections.
@vadmium Thanks for your comments. I hope others chime in. Being fairly new to Python, I've found my chart (or your changes) very helpful in using the right str functions. To your point, it didn't really do much to shorten the page though. |
Doc/library/stdtypes.rst
Outdated
+==========================+==============================+==============================+ | ||
| Formatting | :meth:`str.format` | | | ||
+ +------------------------------+------------------------------+ | ||
| | :meth:`str.format_map` | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe __mod__
(%
) should be included here too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added some lines for the printf-style and for f-strings. I think I linked them to the best places for a description.
Doc/library/stdtypes.rst
Outdated
| Category | str methods | bytes methods | | ||
+==========================+==============================+==============================+ | ||
| Formatting | :meth:`str.format` | | | ||
+ +------------------------------+------------------------------+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the leftmost +
can/should be a |
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Doc/library/stdtypes.rst
Outdated
+ +------------------------------+------------------------------+ | ||
| | :meth:`str.index` | :meth:`bytes.index` | | ||
+ +------------------------------+------------------------------+ | ||
| | :meth:`str.rindex` | :meth:`bytes.rindex` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps the methods and their corresponding rmethods could be combined on the same line to make the table more compact. If however the end result isn't more readable, you could leave them separate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the methods and rmethods on the same lines, but I did it 2 different ways so that you could pick which one you prefer. find\rfind
and index\rindex
are done as columns while split\rsplit
, ljust\rjust
, and lstrip/rstrip
are done as or
.
Doc/library/stdtypes.rst
Outdated
@@ -1464,6 +1566,7 @@ multiple fragments. | |||
sections. In addition, see the :ref:`stringservices` section. | |||
|
|||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this change intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching that! I've removed it.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
I have made the requested changes; please review again |
Thanks for making the requested changes! @ezio-melotti: please review the changes made to this pull request. |
@ezio-melotti Do you review this PR? |
This PR is stale because it has been open for 30 days with no activity. |
This PR is stale because it has been open for 30 days with no activity. |
This is how the (first half of the) table looks like. The table intentionally splits some cells and uses a One issue with the table is that it takes over a page and it has a lot of whitespace. Maybe we could do something that looks like this instead (using a proper colspan):
We lose the direct mapping from |
Hmm, well take my opinion as just one data point, but while I do definitely see the concern of it being rather long, at least from what I can see so far, I find the current format a good deal easier to skim and consult as a structured summary, at least personally to my eyes. If we're going to go with a structure similar to what you propose, I'm not sure if we even need a table at all, as opposed to just a bulleted list, which has essentially the same structure, is even more compact and is simpler to read and write, e.g.
Meanwhile, the original table could be made more compact and maintainable, at least in the source, by formatting it as a Also, of the two styles, FWIW a real cell split rather than a raw |
This PR is stale because it has been open for 30 days with no activity. |
This PR is stale because it has been open for 30 days with no activity. |
@ezio-melotti I'm going through and rebooting and/or closing old Docs PRs. This one looks good and is an improvement over not having it. Would you be cool with merging? There would always be the option to iterate on it in the future. Thanks! |
This PR is stale because it has been open for 30 days with no activity. |
Doc/library/stdtypes.rst
Outdated
+--------------------------+-------------------------------------------+---------------------------------------------------+ | ||
| Splitting and Joining | :meth:`str.split` | :meth:`str.rsplit` | :meth:`bytes.split` | :meth:`bytes.rsplit` | | ||
+--------------------------+-------------------------------------------+---------------------------------------------------+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The r/l pairs (split/rsplit, ljust/rjust, lstrip/rstrip) need +
s to render as table cells:
+--------------------------+-------------------------------------------+---------------------------------------------------+ | |
| Splitting and Joining | :meth:`str.split` | :meth:`str.rsplit` | :meth:`bytes.split` | :meth:`bytes.rsplit` | | |
+--------------------------+-------------------------------------------+---------------------------------------------------+ | |
+--------------------------+-------------------+-----------------------+---------------------+-----------------------------+ | |
| Splitting and Joining | :meth:`str.split` | :meth:`str.rsplit` | :meth:`bytes.split` | :meth:`bytes.rsplit` | | |
+--------------------------+-------------------+-----------------------+---------------------+-----------------------------+ |
I agree that it's an improvement and can be iterated on. |
Based on the original patch by Martin Panter to create a table categorizing str methods.
https://bugs.python.org/issue23560