Add 'name' as argument for index 'to_frame' method by henriqueribeiro · Pull Request #22580 · 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 }})
- closes #xxxx
- tests added / passed
- passes
git diff upstream/master -u -- "*.py" | flake8 --diff - whatsnew entry
Since series to_frame method has a name argument, I believe it makes sense for index also have it.
The problem is that a name argument doesn’t make sense for a MultiIndex. Not sure what’s best here.
| """ |
|---|
| from pandas import DataFrame |
| name = self.name or 0 |
| if not name: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would have to be if name is not None, to allow falsey names.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right! I will change that
@TomAugspurger regarding MultiIndex, I'm not seeing how this name will affect MultiIndex. Can you enlight me please?
Override for MI with plural names?
@henriqueribeiro : Thanks for the contribution! In the future, if you could open an issue before opening the PR, that would allow us to evaluate the idea first before implementing.
@TomAugspurger I cannot understand why some checks are failing. Can you give me some help please?
There was a new release of openpyxl that's incompatible with pandas. It's been worked around on master, so you need to fetch upstream and push your changes.
git fetch upstream
git merge upstream/master
git push origin
| Parameters |
| ---------- |
| index : boolean, default True |
| Set the index of the returned DataFrame as the original Index. |
| name : object, default None |
| The passed name should substitute for the series name (if it has |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'series' -> 'index'
Needs a release note too, for 0.24
@TomAugspurger Regarding the release notes, should I add this on the indexing section of v0.24.0.txt?
lgtm. was there not a referenced issue?
No. This came from dask and that's why I created the pull request immediately.
All green, and since @jreback has already approved this, merging.
aeltanawy pushed a commit to aeltanawy/pandas that referenced this pull request
Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request