-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
DOC: update str.cat example #23723
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
DOC: update str.cat example #23723
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
7982f69
DOC: update str.cat example
h-vetinari 2655e8b
Merge remote-tracking branch 'upstream/master' into str_cat_docs
h-vetinari 75ef297
Review (jreback & WillAyd)
h-vetinari f744052
Review (WillAyd & jreback)
h-vetinari 2180da6
Merge remote-tracking branch 'upstream/master' into str_cat_docs
h-vetinari ccf14a3
Merge and remove Index from example (review jreback)
h-vetinari a381c95
Merge remote-tracking branch 'upstream/master' into str_cat_docs
h-vetinari fc1d2dd
Review (WillAyd)
h-vetinari 5d59dd1
Merge remote-tracking branch 'upstream/master' into str_cat_docs
h-vetinari b75c415
Review (jreback)
h-vetinari 3f8d4fd
Merge remote-tracking branch 'upstream/master' into str_cat_docs
h-vetinari File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Might have missed this but what's the reason for bringing up PandasArray? Not really something the end user would be using directly (at least in current form)
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.
PandasArray
is very user-facing:and the current example was recently changed to use
.array
instead of.values
. I think this should be documented clearly.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.
OK thanks. I have been somewhat on the sidelines for that conversation so I'll defer to @jreback specifically on this piece
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.
You don't need to mention
PandasArray
here, its not very interesting, nor relevant. Just say array-likes. and removeu.array
, theu.to_numpy()
is the corrent idiom here.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.
@jreback
There is a big difference between
u.array
andu.to_numpy()
:I'm guessing
.array
will eventually replace.values
-usage (e.g. to get rid of the index for.str.cat
), since it is by design better suited for pandas-internal dtypes, and so the distinction above is not just an irrelevant detail IMO.I want to show here the explicitly allowed item types to pass into a list-like, which have to pass:
Long story short, I want to show a list-like containing an
np.ndarray
, aPandasArray
(to reiterate, this example was already changed by @TomAugspurger to use.array
instead of.values
in #23623), and aSeries
. (IncludingIndex
would be nice-to-have, but too complicated absent #22225).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.
@h-vetinari
.to_numpy()
replaces.values
;.array
is user-accessible, but generally is not visible to by the user. its not necessary here and is just noise.Uh oh!
There was an error while loading. Please reload this page.
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.
@jreback
As I did in the last few commits...
Will remove, but pinging @TomAugspurger since he added the current
u.array
in this example in #23623 (although likely "just" for replacingu.values
?).