Skip to content

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

Merged
merged 9 commits into from
Jan 13, 2025

Conversation

csabella
Copy link
Contributor

@csabella csabella commented May 22, 2017

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

https://bugs.python.org/issue23560

Copy link
Member

@vadmium vadmium left a 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.
Copy link
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
Contributor Author

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
Copy link
Contributor Author

csabella commented May 27, 2017

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

+==========================+==============================+==============================+
| Formatting | :meth:`str.format` | |
+ +------------------------------+------------------------------+
| | :meth:`str.format_map` | |
Copy link
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
Contributor Author

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.

| Category | str methods | bytes methods |
+==========================+==============================+==============================+
| Formatting | :meth:`str.format` | |
+ +------------------------------+------------------------------+
Copy link
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
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

+ +------------------------------+------------------------------+
| | :meth:`str.index` | :meth:`bytes.index` |
+ +------------------------------+------------------------------+
| | :meth:`str.rindex` | :meth:`bytes.rindex` |
Copy link
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
Contributor Author

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.

@@ -1464,6 +1566,7 @@ multiple fragments.
sections. In addition, see the :ref:`stringservices` section.



Copy link
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
Contributor Author

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

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
Contributor Author

I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

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

@adorilson
Copy link
Contributor

@ezio-melotti Do you review this PR?

@github-actions
Copy link

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

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

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

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Aug 28, 2022
@ezio-melotti ezio-melotti changed the title bpo-23560: DOC:Add summary table for str methods in stdtypes.rst gh-67748: DOC:Add summary table for str methods in stdtypes.rst Aug 30, 2022
@ezio-melotti
Copy link
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
str str.find/str.rfind, str.index/str.rindex, str.startswith/str.endswith, str.count, str.replace
bytes bytes.find/bytes.rfind, bytes.index/bytes.rindex, bytes.startswith/bytes.endswith, bytes.count, bytes.replace
Splitting and Joining
str str.split/str.rsplit, str.splitlines, str.partition/str.rpartition,str.join
bytes bytes.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-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 31, 2022
@CAM-Gerlach CAM-Gerlach added the docs Documentation in the Doc dir label Aug 31, 2022
@CAM-Gerlach
Copy link
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
Member

See also these issues:

@github-actions
Copy link

github-actions bot commented Oct 2, 2022

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

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

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

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label May 27, 2023
@willingc willingc requested a review from ezio-melotti October 31, 2024 02:00
@willingc
Copy link
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-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Oct 31, 2024
Copy link

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

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Nov 30, 2024
Comment on lines 1575 to 1577
+--------------------------+-------------------------------------------+---------------------------------------------------+
| Splitting and Joining | :meth:`str.split` | :meth:`str.rsplit` | :meth:`bytes.split` | :meth:`bytes.rsplit` |
+--------------------------+-------------------------------------------+---------------------------------------------------+
Copy link
Member

@encukou encukou Dec 19, 2024

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
Member

encukou commented Dec 19, 2024

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

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Dec 19, 2024
@encukou encukou merged commit 5044c22 into python:main Jan 13, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

10 participants