Skip to content

FIX printing index with display.max_seq_items=None (GH10182) #10183

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 10, 2015
Merged

FIX printing index with display.max_seq_items=None (GH10182) #10183

merged 2 commits into from
Jun 10, 2015

Conversation

jorisvandenbossche
Copy link
Member

Fixes #10182

@jorisvandenbossche jorisvandenbossche added Bug Output-Formatting __repr__ of pandas objects, to_string labels May 21, 2015
@jorisvandenbossche jorisvandenbossche added this to the 0.17.0 milestone May 21, 2015
@jorisvandenbossche
Copy link
Member Author

@jreback two reasons for the failures (both because I used repeat):

  • CategoricalIndex -> repeat does not work, as Categorical has no repeat method -> do I add this?
  • PeriodIndex -> repeat removes the freq -> this seems like a bug? (or not specifically implemented/checked for PeriodIndex)

@jreback
Copy link
Contributor

jreback commented May 21, 2015

I think if you remove tseries/base/DatetimeIndexOpsMix/repeat the PeriodIndex case will be fixed (as core/index.py/Index/repeat should work; this correctly propogates the meta-data). Obviously not tested though.

I think CategoricalIndex might need its own repeat, though you might be able to 'combine' it with MultiIndex.repeat as these are pretty similar. (You can create a mixin, maybe MultiIndexMixin) or something to hold these (and a couple more methods IIRC).

@jorisvandenbossche
Copy link
Member Author

For the CategoricalIndex, why not adding Categorical.repeat? This seems rather straightforward at first sight, as repeating the codes should do? And then no logic on the Index itself has to be changed.

@jreback
Copy link
Contributor

jreback commented May 21, 2015

@jorisvandenbossche that's what I said, add for CategoricalIndex.repeat (you can combine with MultiIndex.repeat if you want to (side issue)

@jorisvandenbossche
Copy link
Member Author

I mean Categorical.repeat, not CategoricalIndex.repeat

@jreback
Copy link
Contributor

jreback commented May 21, 2015

@jorisvandenbossche oh, yes, for sure do that!

@jorisvandenbossche jorisvandenbossche force-pushed the index-repr-setting branch 3 times, most recently from 6d07d2e to 57a69cd Compare May 28, 2015 08:43
@jorisvandenbossche
Copy link
Member Author

@jreback can you have a look?

jreback
jreback reviewed May 28, 2015
View reviewed changes
pandas/tests/test_index.py
@@ -133,6 +133,14 @@ def test_str(self):
self.assertTrue("'foo'" in str(idx))
self.assertTrue(idx.__class__.__name__ in str(idx))

def test_repr_max_seq_item_setting(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

move this to the Base class which tests for all sub-classes

Copy link
Member Author

Choose a reason for hiding this comment

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

this is already in the Base class

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh, you know what the function above it is not testing much test_str (that's what I was looking at). should be iterating over the self.indices rather than using .create_index().

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not fully following the logic of the test set up.

  • on the one hand, you have the self.indices -> so iterating over these will run this test for all the different index types
  • but create_index is also redefined in each subclass of Base, so in this way the test will also be run for the different index types

So it seems two ways to achieve a bit the same?

Copy link
Contributor

Choose a reason for hiding this comment

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

.indices has much more to test (and is customizable per sub-class), while .create_index is a single plain vanilla one. For the generic tests using .indices provides more coverage.

@jorisvandenbossche jorisvandenbossche modified the milestones: 0.16.2, 0.17.0 Jun 2, 2015
@jreback
Copy link
Contributor

jreback commented Jun 5, 2015

this looked reasonable

jreback
jreback reviewed Jun 9, 2015
View reviewed changes
doc/source/whatsnew/v0.17.0.txt
@@ -63,7 +63,7 @@ Bug Fixes
- Bug where read_hdf store.select modifies the passed columns list when
multi-indexed (:issue:`7212`)
- Bug in ``Categorical`` repr with ``display.width`` of ``None`` in Python 3 (:issue:`10087`)

- Bug in Index repr when using the ``max_seq_items=None`` setting (:issue:`10182`).
Copy link
Contributor

Choose a reason for hiding this comment

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

move to 0.16.2

@jorisvandenbossche
Copy link
Member Author

@jreback yes, coming back to this. I think your main comment here is that the tests can be expanded by looping over the indices. However, as I am rather busy at work this week, is it OK to leave that for another PR? Besides the whatsnew notice I think this is good to go.

@jreback
Copy link
Contributor

jreback commented Jun 9, 2015

@jorisvandenbossche sure. Always like expanded tests, but otherwise ok. merge when ready.

@jorisvandenbossche
Copy link
Member Author

ok, will do this evening

jorisvandenbossche added a commit that referenced this pull request Jun 10, 2015
@jorisvandenbossche
Merge pull request #10183 from jorisvandenbossche/index-repr-setting
ba69a49 
FIX printing index with display.max_seq_items=None (GH10182)
@jorisvandenbossche jorisvandenbossche merged commit ba69a49 into pandas-dev:master Jun 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Bug Output-Formatting __repr__ of pandas objects, to_string
Projects
None yet
Milestone
0.16.2
Development

Successfully merging this pull request may close these issues.

Setting option 'display.max_seq_items' to None results in TypeError when pprinting index
2 participants
@jorisvandenbossche @jreback