-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
fixed bug where pd.NA was being cast to NaN during formatting #55754
Conversation
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.
Looking good, can you add a test.
passed new test locally
im not sure, is it normal that so many CI steps are failing? (it's my first time contributing to pandas) |
Two things to do when the CI is failing: look at the failure message of (some) builds and see if the builds are passing on main. For the first, I'm seeing: https://github.com/pandas-dev/pandas/actions/runs/6703704571/job/18214736331?pr=55754#step:4:226
This is likely not due to your PR 😄 Recent commits to main also have 4 failing. Once you believe the failed builds are not due to your PR and you're not seeing them fail on the most recent commit to main, try merging main into this branch. It will run all the builds again. |
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.
Looking good!
pandas/io/formats/format.py
Outdated
self.tr_frame = self.tr_frame.iloc[ | ||
chain(range(row_num), range(_len - row_num, _len)) | ||
] |
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.
Thanks - this is definitely a lot better. I think we can do a little better by using NumPy instead.
self.tr_frame = self.tr_frame.iloc[
np.hstack([np.arange(row_num), np.arange(_len - row_num, _len)])
]
@@ -171,6 +171,15 @@ def test_repr_truncation(self): | |||
with option_context("display.max_colwidth", max_len + 2): | |||
assert "..." not in repr(df) | |||
|
|||
def test_repr_truncation_preserves_na(self): | |||
# https://github.com/pandas-dev/pandas/issues/55630 | |||
with option_context("display.max_rows", 10): |
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.
If you set this to 2 instead of 10, I think we could then test for the entire string.
it looks like main is having the same CI failures as here |
Looks good - I think CI issues will be resolved when you merge main. |
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.
LGTM merge when ready @rhshadrach
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.
lgtm
Thanks @dominiquegarmier! |
replaced
pd.concat
withDataFrame.drop
to copypd.NA
more faithfully (see linked issue)pd.NA
not printed properly in REPL when DataFrame gets collapsed #55630