Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-106318: Add examples for thestr methods in collapsible sections#111743

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
adorilson wants to merge55 commits intopython:main
base:main
Choose a base branch
Loading
fromadorilson:str_methods_collapsible

Conversation

adorilson
Copy link
Contributor

@adorilsonadorilson commentedNov 4, 2023
edited by bedevere-appbot
Loading

This PR complements#105670, turning the examples into collapsible sections, as suggested in#106318.

I would like a "go ahead" to continue the work.


📚 Documentation preview 📚:https://cpython-previews--111743.org.readthedocs.build/

Copy link
Contributor

@hauntsaninjahauntsaninja left a comment

Choose a reason for hiding this comment

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

Reviewed up untilformat_map, since beyond the PR doesn't yet usedetails

adorilson reacted with thumbs up emoji
Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
Copy link
Member

@ezio-melottiezio-melotti left a comment

Choose a reason for hiding this comment

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

As I mentionedin the issue, I don't think using.. raw:: is a good approach.

An "Examples" section at the bottom would IMHO be better, and could also combine related functions (e.g.upper/lower/title/capitalize/casefold) like I did inthestr.format docs.

@hauntsaninja
Copy link
Contributor

hauntsaninja commentedJan 23, 2024
edited
Loading

Shipping these examples somewhere else would be worse for users IMO. If I want to look up what str.maketrans does, I'm going to look at the str.maketrans docs, not read through the whole page

In these discussion about.. raw:: I'm having a hard time understanding whether objections are fundamental to the proposal or whether they're predicated on sphinx implementation issues. If we had a magic sphinx wand, are inline collapsible sections something you (or other docs-community folks) would be okay with or not?

AlexWaygood reacted with thumbs up emojiadorilson reacted with heart emoji

@ezio-melotti
Copy link
Member

If I want to look up what str.maketrans does, I'm going to look at the str.maketrans docs, not read through the whole page

This is a discoverability problem, but it can be easily be solved by turning the "See examples" text that expands the collapsed section in a link to the bottom of the page, where the section with all the examples is.

In these discussion about .. raw:: I'm having a hard time understanding whether objections are fundamental to the proposal or whether they're predicated on sphinx implementation issues.

There are a few issues at different levels:

  • .. raw:: itself is not an elegant solution implementation-wise, since it requires hardcoding raw HTML and also leads to repetition and increases the risk of errors (I'm not sure Sphinx is able to check markup errors inraw directives).
  • using existing plugins solves these problems, but introduces a new issue:the doc should be buildable without third-part extensions.
  • both these solutions, afaik, have the additional problem that they only works with the HTML builder, and not with the other builders -- something that we also want to support (adopting graceful degradation is an acceptable compromise here).

If we had a magic sphinx wand, are inline collapsible sections something you (or other docs-community folks) would be okay with or not?

If all the problems listed above were magically solved (i.e. if Sphinx added a built-in support for collapsible sections that words with all builders), then they could definitely be a useful tool. They still have some minor issues (e.g. iirc ctrl+f doesn't detect text within a collapsed section and they might be easy to miss while skimming through a page), but at that point the pros might outweigh the cons.

Finally, there's also another higher-level and more general discussion that we should have (probably not here, the next doc meeting might be better): should we have some convention on where to put examples?

We don't want to end up in a situation where a page uses collapsible sections, another hasexamples at the top, anotherat the bottom, anotherin the middle, anotherin a separate page, etc. If we agree on some conventions, it will also be easier for users to set their expectations and know where to look.

.. raw:: html

</details>
</dd>


.. _meth-str-join:
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Is this a useful directive? If yes, can I add to other methods in this PR or another?

@ezio-melotti
Copy link
Member

Finally, there's also another higher-level and more general discussion that we should have (probably not here, the next doc meeting might be better): should we have some convention on where to put examples?

We discussed this duringthe last meeting, and we agreed that a better approach would be to have a couple of simpler/short examples inline (i.e. just after the fuction doc) and possibly link to an "Example" section with more examples at the bottom. If we follow this approach, we don't need to worry about having collapsible sections and the issues that come along with those.

@hauntsaninja
Copy link
Contributor

hauntsaninja commentedFeb 11, 2024
edited
Loading

Great! The examples in this PR are all simple and short.

So it sounds like we should just keep these in line, without the collapsible sections. And as per your last comment, if Sphinx develops better collapsible section tech in the future, we can incorporate this (and I'm happy to do the work for this myself).

In case folks read this thread later want a concrete example of what that would look like, here's the equivalent Go docs with collapsible sections:https://pkg.go.dev/strings#pkg-functions (to keep in theme with theRob Pike quote from above )

ezio-melotti reacted with thumbs up emoji

@adorilson
Copy link
ContributorAuthor

So it sounds like we should just keep these in line, without the collapsible sections. And as per your last comment, if Sphinx develops better collapsible section tech in the future, we can incorporate this (and I'm happy to do the work for this myself).

Do you mean reject this PR in favor of#105670 ?

@ezio-melotti
Copy link
Member

Do you mean reject this PR in favor of#105670 ?

Are those the same examples, just without collapsible sections?

@adorilson
Copy link
ContributorAuthor

Do you mean reject this PR in favor of#105670 ?

Are those the same examples, just without collapsible sections?

Exactly. For this PR, I only forked that branch and added the collapsible sections.

ezio-melotti and blaisep reacted with thumbs up emoji

@hugovkhugovk removed the needs backport to 3.11only security fixes labelApr 11, 2024
@serhiy-storchakaserhiy-storchaka added the needs backport to 3.13bugs and security fixes labelMay 9, 2024
@blaisep
Copy link
Contributor

OK, I have been reviewing "simple" PR docs and it seems like the consensus is to use#105670 and then close this one (#111743 )

@adorilson , if you agree, please close this PR.

adorilson reacted with eyes emoji

@hauntsaninja
Copy link
Contributor

hauntsaninja commentedMay 22, 2024
edited
Loading

Silly question, where is the consensus?

@blaisep
Copy link
Contributor

Silly question, where is the consensus?

Well, it seems that@adorilson ,@ezio-melotti ,@hugovk ,@terryjreedy ,@CAM-Gerlach ,@encukou all agree that the examples are useful and that we should use them without the additional formatting.
@hauntsaninja , do you have an objection to adding the examples?

encukou reacted with thumbs up emoji

@hauntsaninja
Copy link
Contributor

hauntsaninja commentedMay 23, 2024
edited
Loading

I'm strongly in favour of adding the examples, see e.g.#111743 (review) and my other reviews on this and related PRs

But last I checked, Ezio and others weren't sure what the best way and place and format to add them was. I still can't find discussion that comes to a consensus on this (i.e. to go with#105670) and was curious, which is why I asked :-)

(Personally, I'm fine with any option that adds the examples, with a very mild preference for collapsible sections)

AlexWaygood reacted with thumbs up emoji

@adorilson
Copy link
ContributorAuthor

But last I checked, Ezio and others weren't sure what the best way and place and format to add them was. I still can't find discussion that comes to a consensus on this (i.e. to go with#105670) and was curious, which is why I asked :-)

In fact, maybe "consensus" is a strong word.
But I understood that adding examples without collapsible sections was a decision, and@ezio-melotti would review#105670 .

@blaisep
Copy link
Contributor

blaisep commentedMay 24, 2024
edited
Loading

I see. Well, I think the examples are very good, so I made a bit of a compromise by trying to integrate them more closely with the prose they describe,Doc/library/stdtypes.rst
My motivation is to expedite getting these examples into the docs.
If PR#119445 is not useful, I'll close it.

adorilson reacted with heart emoji

@adorilson
Copy link
ContributorAuthor

Hi,@blaisep.

I've seen your changes on my PR. And I like it.

I'll keep this PR open until some other related PR is merged. But if some core developers want to close it, go ahead. I think this (and also#105670 ) can be closed in favour of#119445.

blaisep reacted with thumbs up emojiblaisep reacted with rocket emoji

@picnixz
Copy link
Member

picnixz commentedMay 28, 2024
edited
Loading

By the way, we are currently in the process of revivingsphinx-doc/sphinx#10532 which could make this PR much cleaner in the future. I cannot give you a merge ETA because Adam and Chris seem quite busy but I personally hope that we can make it for the next Sphinx release (or the one after).

EDIT: I know that this PR was already mentioned but since it was in January and we had some activity in April, I wanted to mention that we did not forget the use-case.

adorilson reacted with thumbs up emojihauntsaninja reacted with rocket emoji

@hugovkhugovk removed the needs backport to 3.12only security fixes labelApr 10, 2025
@serhiy-storchakaserhiy-storchaka added the needs backport to 3.14bugs and security fixes labelMay 8, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@hauntsaninjahauntsaninjahauntsaninja left review comments

@ezio-melottiezio-melottiezio-melotti left review comments

Assignees
No one assigned
Labels
awaiting reviewdocsDocumentation in the Doc dirneeds backport to 3.13bugs and security fixesneeds backport to 3.14bugs and security fixesskip news
Projects
Status: Todo
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

8 participants
@adorilson@AA-Turner@hugovk@hauntsaninja@ezio-melotti@blaisep@picnixz@serhiy-storchaka

[8]ページ先頭

©2009-2025 Movatter.jp