Skip to content

Conversation

@Dr-Irv
Copy link
Contributor

@Dr-Irv Dr-Irv commented Feb 22, 2017

See discussion in #10461 . Now if we internally know we need to call get_level_values for a specific level number, there is an internal function for doing that, which should allow better support for when people create indices with names that are integers.

@Dr-Irv Dr-Irv closed this Feb 22, 2017
@jreback
Copy link
Contributor

jreback commented Feb 23, 2017

did u mean to close?
this looks pretty reasonable

@Dr-Irv
Copy link
Contributor Author

Dr-Irv commented Feb 23, 2017

@jreback yes. The conflicts indicated you had made a change that was incompatible with what I did, so I need to fix that. Will create a new pull request when ready.

@jreback
Copy link
Contributor

jreback commented Feb 23, 2017

@Dr-Irv you don't need to close a PR to fix that though. simply rebase and force push. you never need to open a new PR for the same exact issue.

@Dr-Irv
Copy link
Contributor Author

Dr-Irv commented Feb 23, 2017

@jreback OK, didn't know that. Should I reopen this one when I'm ready?

@jreback
Copy link
Contributor

jreback commented Feb 23, 2017

sure

@jorisvandenbossche
Copy link
Member

@Dr-Irv Already reopened it, because if you push again while the PR is closed, you cannot reopen it anymore.

@jreback jreback added Bug MultiIndex Output-Formatting __repr__ of pandas objects, to_string labels Feb 23, 2017
- ``Series/DataFrame.resample.asfreq`` have gained a ``fill_value`` parameter, to fill missing values during resampling (:issue:`3715`).
- ``pandas.tools.hashing`` has gained a ``hash_tuples`` routine, and ``hash_pandas_object`` has gained the ability to hash a ``MultiIndex`` (:issue:`15224`)
- ``Series/DataFrame.squeeze()`` have gained the ``axis`` parameter. (:issue:`15339`)
- ``Series/DataFrame.squeeze()`` have gained the ``axis`` parameter. (:issue:`15339`)<<<<<<< f4edb053e17e51e8c2bed7c16755c4f7f3222117
Copy link
Contributor

Choose a reason for hiding this comment

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

merge residual :>

- HTML table output skips ``colspan`` or ``rowspan`` attribute if equal to 1. (:issue:`15403`)
- ``pd.TimedeltaIndex`` now has a custom datetick formatter specifically designed for nanosecond level precision (:issue:`8711`)
- ``pd.types.concat.union_categoricals`` gained the ``ignore_ordered`` argument to allow ignoring the ordered attribute of unioned categoricals (:issue:`13410`). See the :ref:`categorical union docs <categorical.union>` for more information.
- Using numerical names in ``MultiIndex`` causes less errors. (:issue:`12223`) (:issue:`15262`)
Copy link
Contributor

Choose a reason for hiding this comment

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

say instead about the bug report about output formatting with a MI under certain conditions.

Copy link
Contributor

Choose a reason for hiding this comment

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

(:issue:`12223`, :issue:`15262`)

return self

def _get_level_values(self, num):
# Used to mirror implementation for MultiIndex
Copy link
Contributor

Choose a reason for hiding this comment

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

better to add an actual doc-string

Parameters
----------
level : int level
copy : bool whether copy of results should be done
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you adding this? this is a whole different ball game.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jreback What I needed _get_level_values() to do is have the same behavior as the public get_level_values(), with the assumption of the int argument, so the copy argument makes that happen by doing the shallow copy there.

When I first looked at this, there was no _get_level_values(), but that got introduced within the past 2 weeks, so I then had to make everything compatible.

Copy link
Contributor

Choose a reason for hiding this comment

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

still its not clear why you would actually be making this change, it just adds too much complexity.

show an example of why you think you need it (or simply take it out)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jreback I'll try an alternate implementation and let you review that.

@codecov-io
Copy link

codecov-io commented Feb 23, 2017

Codecov Report

Merging #15478 into master will increase coverage by <.01%.
The diff coverage is 88.23%.

@@ Coverage Diff @@ ## master #15478 +/- ## ========================================== + Coverage 90.36% 90.36% +<.01%  ========================================== Files 135 135 Lines 49519 49522 +3 ========================================== + Hits 44747 44750 +3  Misses 4772 4772
Impacted Files Coverage Δ
pandas/io/sql.py 94.79% <ø> (ø)
pandas/util/doctools.py 0% <ø> (ø)
pandas/indexes/base.py 96.24% <100%> (ø)
pandas/indexes/multi.py 96.65% <100%> (ø)
pandas/core/groupby.py 94.98% <100%> (ø)
pandas/core/frame.py 97.83% <100%> (ø)
pandas/core/reshape.py 99.25% <100%> (ø)
pandas/formats/format.py 95.31% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b94186d...15d8433. Read the comment docs.

@Dr-Irv
Copy link
Contributor Author

Dr-Irv commented Feb 24, 2017

@jreback the last commit I did should have addressed the changes you requested, and we're all green, so let me know if there is more to do

@jreback jreback added this to the 0.20.0 milestone Feb 24, 2017
@jreback jreback closed this in 5955804 Feb 24, 2017
@jreback
Copy link
Contributor

jreback commented Feb 24, 2017

thanks @Dr-Irv

@jorisvandenbossche
Copy link
Member

@Dr-Irv Thanks!

@Dr-Irv Dr-Irv deleted the Issue15262 branch February 28, 2017 15:43
AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this pull request Mar 21, 2017
…n MultiIndex closes pandas-dev#12223 closes pandas-dev#15262 Author: Dr-Irv <irv@princeton.com> Closes pandas-dev#15478 from Dr-Irv/Issue15262 and squashes the following commits: 15d8433 [Dr-Irv] Address jreback comments 10667a3 [Dr-Irv] Fix types for test 8935068 [Dr-Irv] resolve conflicts 385ca3e [Dr-Irv] BUG: GH pandas-dev#12223, GH pandas-dev#15262. Allow ints for names in MultiIndex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug MultiIndex Output-Formatting __repr__ of pandas objects, to_string

4 participants