-
-
Notifications
You must be signed in to change notification settings - Fork 18k
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
REF: handling of max_colwidth parameter #25977
Conversation
can you merge master and update |
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.
looks good, can you add a whatsnew note
@@ -873,6 +875,11 @@ def format_array(values, formatter, float_format=None, na_rep='NaN', | |||
When formatting an Index subclass | |||
(e.g. IntervalIndex._format_native_types), we don't want the | |||
leading space since it should be left-aligned. | |||
max_colwidth: False, int or None, optional, default None |
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.
can you add a versionadded tag
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.
not a public function. have added anyway.
@simonjayhawkins can you rebase and update when possible |
@simonjayhawkins whats the status of this? |
still need to do a precursor. checks were passing and i don't think they should have been. so holding as draft unless you'd rather it be closed for now.
comments addressed.
this should be a pure refactor. no whatsnew needed? |
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.
looks fine. small comments.
@@ -420,6 +428,24 @@ def _get_formatter(self, i): | |||
i = self.columns[i] | |||
return self.formatters.get(i, None) | |||
|
|||
def _format_col(self, i: int): | |||
""" | |||
Calls `format_array` for column `i` of truncated DataFrame with |
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.
can you say positional column i
* int: the maximum width of strings | ||
* None: use display.max_colwidth setting | ||
|
||
.. versionadded:: 1.0 |
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.
can just remove the versionadded (or make it 0.25.0)
@simonjayhawkins is this still active? |
can close for now. |
git diff upstream/master -u -- "*.py" | flake8 --diff
this would be a precursor to fixes for #7059, #9784 and #6491.
getting the display.max_colwidth value is moved from
_make_fixed_width
toformat_array
to be consistent with the handling of thecolumn_space
,float_format
andprecision
display options.the PR also includes refactoring of the prior fix to
to_html
#24841_format_col
is moved fromDataFrameFormatter
toTableFormatter
to make it available toHTMLFormatter
andNotebookFormatter
.at the moment
_make_fixed_width
is called byformat_array
, but also by_to_str_columns
and_get_formatted_index
._to_str_columns
is not an issue because the calls are preceded to calls toformat_array
IMO
_get_formatted_index
needs to be refactored to callformat_array
directly and probably some additional tests as a precursor to this PR.thoughts?
cc @TomAugspurger