-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Can't combine DatetimeIndex MultiIndex with numeric MultiIndex #42143
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
Conversation
Alex-JG3
commented
Jun 20, 2021
•
edited by jreback
Loading
edited by jreback
- closes Can't combine DatetimeIndex MultiIndex with numeric MultiIndex #23478
- tests added / passed
- Ensure all linting tests pass, see here for how to run them
- whatsnew entry
for columns in columns_list | ||
] | ||
|
||
for i, j in combinations(range(len(dfs)), 2): |
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 parameterize instead of doing thsi.
pls check the warning text as well.
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 for the comment. I have attempted to parameterize the test as requested. I suspect I am not using assert_produces_warning
correctly, could you give any hints how I should correctly check for the TypeErrors?
@AlexGregory96 are there other concat tests in this file? I think
|
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.
a file path_to_file.xlsx is included somehow, can you remove. otherwise lgtm.
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this. |
this needs a rebase |
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this. |
pls merge master and address comments |
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. CI failure unrelated
thanks @Alex-Gregory-1 |