DOC/CLN: Revise StringMethods docs by sinhrks · Pull Request #9843 · pandas-dev/pandas (original) (raw)
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Derived from #9773, #9667. Fix docstrings to meet what current .str accessors does.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this wording it sounds as all the strings in the series are concatenated
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, but this is also the case ..
This is quite a different behaviour when having others or not. Maybe some more explanation or an example would be useful.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it is better. Added an example.
@jorisvandenbossche Thank you for your comments.
Thanks to #9667, Index now support .str accessor. I've tried to describe the Series/Index return values but may not be clear because it shares the same docstring...
CC: @mortada Can you check if you have a chance?
(WIP) DOC/CLN: Revise StringMethods docs DOC/CLN: Revise StringMethods docs
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the HTML doc will append () and this would show up as "equivalent to str.startswith(pat)()", probably best to remove the (pat) at the end
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or it would even error, I am not sure. But indeed, just use str.startswith here
@sinhrks spotted a few minor things (please see comments inline) but otherwise looks good to me, thanks!
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be If others is specified ... ?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But nice examples!
@sinhrks One more general comment: you now used Series``/``Index almost everywhere, but I think in general we don't quote Series or DataFrame in our docstrings.
Also in the docstrings here you used both forms (eg unquoted in the parameter explanations or returns section).
I don't think we have a strict rule about this (and in the tutorial docs it is a bit mixed as well), but maybe we should keep it unquoted in the docstring? Opinion?
@jorisvandenbossche , @mortada Thanks for the comments. I've fixed the docs based on the points.
About quotes:
I also didn't have a fixed policy, only an impression that no need to quote return values because these are rendered as italic. I agree removing quote is more consistent. How should array be (remove or quote)?
jorisvandenbossche added a commit that referenced this pull request
DOC/CLN: Revise StringMethods docs
Thanks! Always good to have more work on the docs
Thanks for review! Lmk if needs any follow-ups.
Labels
String extension data type and string data