Skip to content

gh-67748: DOC:Add summary table for str methods in stdtypes.rst - #1709

Merged
encukou merged 9 commits into
python:mainfrom
csabella:bpo23560
Jan 13, 2025
Merged

gh-67748: DOC:Add summary table for str methods in stdtypes.rst#1709
encukou merged 9 commits into
python:mainfrom
csabella:bpo23560

Conversation

@csabella

@csabellacsabella commented May 22, 2017

Copy link
Copy Markdown
Contributor

Based on the original patch by Martin Panter to create a table categorizing str methods.

https://bugs.python.org/issue23560

@vadmiumvadmium left a comment

Copy link
Copy Markdown
Member

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.

Comment threadDoc/library/stdtypes.rst Outdated

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
ContributorAuthor

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.

@csabella

csabella commented May 27, 2017

Copy link
Copy Markdown
ContributorAuthor

@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.

Comment threadDoc/library/stdtypes.rst Outdated
+==========================+==============================+==============================+
| Formatting | :meth:`str.format` | |
+ +------------------------------+------------------------------+
| | :meth:`str.format_map` | |

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
ContributorAuthor

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.

Comment threadDoc/library/stdtypes.rst Outdated
| Category | str methods | bytes methods |
+==========================+==============================+==============================+
| Formatting | :meth:`str.format` | |
+ +------------------------------+------------------------------+

Copy link
Copy Markdown
Member

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 |.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Thanks!

Comment threadDoc/library/stdtypes.rst Outdated
+ +------------------------------+------------------------------+
| | :meth:`str.index` | :meth:`bytes.index` |
+ +------------------------------+------------------------------+
| | :meth:`str.rindex` | :meth:`bytes.rindex` |

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
ContributorAuthor

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.

Comment threadDoc/library/stdtypes.rst Outdated
sections. In addition, see the :ref:`stringservices` section.



Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Was this change intentional?

Copy link
Copy Markdown
ContributorAuthor

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.

@bedevere-bot

Copy link
Copy Markdown

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. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@csabella

Copy link
Copy Markdown
ContributorAuthor

I have made the requested changes; please review again

@bedevere-bot

Copy link
Copy Markdown

Thanks for making the requested changes!

@ezio-melotti: please review the changes made to this pull request.

@adorilson

Copy link
Copy Markdown
Contributor

@ezio-melotti Do you review this PR?

@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actionsgithub-actionsBot added the stale Stale PR or inactive for long period of time. label Feb 19, 2022
@github-actionsgithub-actionsBot removed the stale Stale PR or inactive for long period of time. label Jul 28, 2022
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actionsgithub-actionsBot added the stale Stale PR or inactive for long period of time. label Aug 28, 2022
@ezio-melottiezio-melotti changed the title bpo-23560: DOC:Add summary table for str methods in stdtypes.rstgh-67748: DOC:Add summary table for str methods in stdtypes.rstAug 30, 2022
@ezio-melotti

Copy link
Copy Markdown
Member

This is how the (first half of the) table looks like. The table intentionally splits some cells and uses a | on others to show two alternative styles.

image

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):

Searching and Replacing
strstr.find/str.rfind, str.index/str.rindex, str.startswith/str.endswith, str.count, str.replace
bytesbytes.find/bytes.rfind, bytes.index/bytes.rindex, bytes.startswith/bytes.endswith, bytes.count, bytes.replace
Splitting and Joining
strstr.split/str.rsplit, str.splitlines, str.partition/str.rpartition,str.join
bytesbytes.split/bytes.rsplit, bytes.splitlines, bytes.partition/bytes.rpartition, bytes.join

We lose the direct mapping from str to the equivalent bytes method, but since they are still grouped it's easy enough to find it.

@github-actionsgithub-actionsBot removed the stale Stale PR or inactive for long period of time. label Aug 31, 2022
@CAM-GerlachCAM-Gerlach added the docs Documentation in the Doc dir label Aug 31, 2022
@CAM-Gerlach

Copy link
Copy Markdown
Member

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.

  • Searching and Replacing
    • str.find/str.rfind, str.index/str.rindex, str.startswith/str.endswith, str.count, str.replace
    • bytes.find/bytes.rfind, bytes.index/bytes.rindex, bytes.startswith/bytes.endswith, bytes.count, bytes.replace
  • Splitting and Joining
    • str.split/str.rsplit, str.splitlines, str.partition/str.rpartition, str.join
    • bytes.split/bytes.rsplit, bytes.splitlines, bytes.partition/bytes.rpartition, bytes.join

Meanwhile, the original table could be made more compact and maintainable, at least in the source, by formatting it as a list-table rather than a full literal table.

Also, of the two styles, FWIW a real cell split rather than a raw | looks a lot cleaner and clearer to me.

@ezio-melotti

Copy link
Copy Markdown
Member

See also these issues:

@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actionsgithub-actionsBot added the stale Stale PR or inactive for long period of time. label Oct 2, 2022
@github-actionsgithub-actionsBot removed the stale Stale PR or inactive for long period of time. label Apr 26, 2023
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actionsgithub-actionsBot added the stale Stale PR or inactive for long period of time. label May 27, 2023
@willingc

Copy link
Copy Markdown
Contributor

@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!

@github-actionsgithub-actionsBot removed the stale Stale PR or inactive for long period of time. label Oct 31, 2024
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actionsgithub-actionsBot added the stale Stale PR or inactive for long period of time. label Nov 30, 2024
Comment threadDoc/library/stdtypes.rst Outdated
Comment on lines +1575 to +1577
+--------------------------+-------------------------------------------+---------------------------------------------------+
| Splitting and Joining | :meth:`str.split` | :meth:`str.rsplit` | :meth:`bytes.split` | :meth:`bytes.rsplit` |
+--------------------------+-------------------------------------------+---------------------------------------------------+

@encukouencukouDec 19, 2024

Copy link
Copy Markdown
Member

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:

Suggested change
+--------------------------+-------------------------------------------+---------------------------------------------------+
|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` |
+--------------------------+-------------------+-----------------------+---------------------+-----------------------------+

@encukou

Copy link
Copy Markdown
Member

I agree that it's an improvement and can be iterated on.
If there are no objections, I'll merge early next year.

@github-actionsgithub-actionsBot removed the stale Stale PR or inactive for long period of time. label Dec 19, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docsDocumentation in the Doc dirskip news

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

10 participants

@csabella@bedevere-bot@adorilson@ezio-melotti@CAM-Gerlach@willingc@encukou@vadmium@the-knights-who-say-ni@arhadthedev