Skip to content
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

Applying max_colwidth to the DataFrame index (#7856) #8954

Closed
wants to merge 7 commits into from
Closed

Applying max_colwidth to the DataFrame index (#7856) #8954

wants to merge 7 commits into from

Conversation

tiagoantao
Copy link
Contributor

closes #7856

reports a problem with the application of max_colwidth to groupby.

The real problem is that max_colwidth is not applied to the index. This patch solves that.

I am not applying self.justify to the formatting of the index (as that would break a test). But maybe it makes more sense to change the test?

justification to comply with tests
@jreback
Copy link
Contributor

jreback commented Dec 2, 2014

needs a test

@jreback jreback added Bug Output-Formatting __repr__ of pandas objects, to_string labels Dec 2, 2014
@jreback jreback added this to the 0.16.0 milestone Dec 2, 2014
@jorisvandenbossche
Copy link
Member

cc @bjonen @onesandzeroes

@tiagoantao
Copy link
Contributor Author

I have added a test case. Tell me if you want anything changed, or if anything is missing. I will do a final squash when everything is up to standard.

@tiagoantao
Copy link
Contributor Author

@jorisvandenbossche I now have time time to finalize this. I have merged with the current head. If there is still interest in dealing with this issue just say. I could imagine that there is still a few things to do (like adding an entry to whatsnew)...

@jorisvandenbossche
Copy link
Member

@tiagoantao Certainly interest!

Remark on the test: I don't think it is testing the correct thing (but you can also leave it in as it is now for testing the width of the columns), as it is testing the width of the columns which is already working. You can eg set some columns as the index, and then test again. That is what now is failing:

In [25]: pd.set_option('max_colwidth', 20)

In [26]: df
Out[26]: 
     a    b                    c  d
0  foo  bar  uncomfortably lo...  1
1  foo  bar                stuff  1

In [27]: df.set_index(['a', 'b', 'c'])  <-- failing in the index
Out[27]: 
                                                    d
a   b   c                                            
foo bar uncomfortably long line with lots of stuff  1
        stuff                                       1

@@ -4795,6 +4795,22 @@ def test_repr_unicode(self):
result = repr(df)
self.assertEqual(result.split('\n')[0].rstrip(), ex_top)

def test_str_max_colwidth(self):
curr_max_colwidth = pd.get_option('max_colwidth')
# As we change a global option, we save it
Copy link
Contributor

Choose a reason for hiding this comment

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

pls add the issue number as a comment here

@jreback
Copy link
Contributor

jreback commented Mar 5, 2015

pls add a release note that explains this. if you can do this asap i can put in 0.16.0

@jorisvandenbossche
Copy link
Member

And the test needs an addition, see my comment above.

@tiagoantao
Copy link
Contributor Author

I will do this over the weekend.

On 6 March 2015 at 09:21, Joris Van den Bossche notifications@github.com
wrote:

And the test needs an addition, see my comment above.

Reply to this email directly or view it on GitHub
#8954 (comment).

When the senses are shaken, and the soul is driven to madness, Who can
stand? - William Blake

@jreback jreback modified the milestones: 0.16.1, 0.16.0 Mar 7, 2015
@tiagoantao
Copy link
Contributor Author

I have tried to sort this out. My sincere apologies for the delay. I will be monitoring this during the next day or so. I will try to react as fast as possible if you find anything in need of intervention.

'd': 1},
{'a': 'foo', 'b': 'bar', 'c': 'stuff', 'd': 1}])
df.set_index(['a', 'b', 'c'])
assert(str(df) == ' a b c d\n'
Copy link
Contributor

Choose a reason for hiding this comment

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

use self.assertTrue(.....) instead of bare assert

@tiagoantao
Copy link
Contributor Author

Did all the corrections. I just hope the way I committed (i.e. in terms of logs and squash) is acceptable. My git-fu is a bit substandard.

I apologize for all these mistakes. I hope in the future, with this experience, I am able to do a few contributions will less noise.

@jreback
Copy link
Contributor

jreback commented Apr 28, 2015

merged via e4cb0f8

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Output-Formatting __repr__ of pandas objects, to_string
Projects
None yet
Development

Successfully merging this pull request may close these issues.

set_option('max_colwidth', N) not working on groupby output
3 participants