Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
bug fix related #5479#6047
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
@@ -410,7 +410,13 @@ def _do_cell_alignment(self): | |||
def auto_set_column_width(self, col): | |||
self._autoColumns.append(col) | |||
# col is a list of column index | |||
if isinstance(col, list): |
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.
This is probably better to do as
try:self._autoColumns.extend(cell)exceptCORRECT_EXCEPTION:self._autoColumns.append(col)
I have change the format from type checking to exception checking. table.auto_set_column_width(-1)# Default inputtable.auto_set_column_width([-1,0,1])# List inputtable.auto_set_column_width((-1,0,1))# Tuple input |
except (TypeError, AttributeError): | ||
self._autoColumns.append(col) | ||
else: | ||
for cell in col: |
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.
This indentation is funny and you can useextend
.
I have just seen the comment from#6059. I am going to add the specific test case for this PR. |
@tacaswell , |
Strings are also iterable. Should that be also supported? |
@dashed , inside the |
@ryanbelt Ah ok. Then should there be a test case for the behaviour you've just described? |
@dashed , thank you for the tips. Unexpected test case for iterable input asstring has been added for this bug. |
@@ -410,7 +410,15 @@ def _do_cell_alignment(self): | |||
def auto_set_column_width(self, col): | |||
self._autoColumns.append(col) | |||
# check for col possibility on iteration |
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.
This might be worded in another way. Suggestion:
If
col
is iterable, concatenate to self._autoColumns. Otherwise, append it to self._autoColumns.
@ryanbelt Awesome. 👍 Unfortunately you beat us to the punch on this PR for d01. But I'll throw some suggestions your way for this PR 😄 . |
FIX: table auto_column_widthfix#5479
I'll leave that for@ryanbelt 😄 |
"""Given column indexs in either List, Tuple or int. Will be able to autonatically set the columns into optimal sizes. """ @tacaswell , |
@ryanbelt Make a new pull request. They are (more-or-less) free and the smaller the change, the easier it is to review. Could you also explain themeaning of the values that are expected to be passed in? |
When iterables were added inmatplotlib#6047, the test added a string. Typingcorrectly points out that that is not accepted, and in fact it does notdo anything (as shown in the test image) because column keys are ints,not strings.
When iterables were added inmatplotlib#6047, the test added a string. Typingcorrectly points out that that is not accepted, and in fact it does notdo anything (as shown in the test image) because column keys are ints,not strings.
Table: auto_set_column_width not working#5479
As before, it append the whole parameter(col) into the
_autoColumns
evencol is a list object.attable.py line 490 inside function
_update_position
_auto_set_column_width
are not able to manipulate with the index number as list rather then int passed from line490.So, I decide to append number to _autoColumns as usual if and only if the parameter is integer. If it is a list, we will append each integer inside the parameter into
_autoColumns