-
Notifications
You must be signed in to change notification settings - Fork 653
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
[FIX] Fix #1683 - losing index names in pd.concat #1684
Conversation
Signed-off-by: Dmitry Chigarev <dmitry.chigarev@intel.com>
Signed-off-by: Dmitry Chigarev <dmitry.chigarev@intel.com>
Codecov Report
@@ Coverage Diff @@
## master #1684 +/- ##
===========================================
- Coverage 84.10% 73.72% -10.38%
===========================================
Files 77 77
Lines 7950 7988 +38
===========================================
- Hits 6686 5889 -797
- Misses 1264 2099 +835
Continue to review full report at Codecov.
|
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 @dchigarev, left some comments for you.
Signed-off-by: Dmitry Chigarev <dmitry.chigarev@intel.com>
Signed-off-by: Dmitry Chigarev <dmitry.chigarev@intel.com>
Signed-off-by: Dmitry Chigarev <dmitry.chigarev@intel.com>
Signed-off-by: Dmitry Chigarev <dmitry.chigarev@intel.com>
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.
@dchigarev Looks good, I would prefer one change for maintainability/simplicity. It might be good to add more specific type hints to the _determine_name
function for the list
portion.
from typing import List
...
def _determine_name(objs: List[QueryCompiler], axis: int):
...
Signed-off-by: Dmitry Chigarev <dmitry.chigarev@intel.com>
Signed-off-by: Dmitry Chigarev <dmitry.chigarev@intel.com>
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. @devin-petersohn , @amyskov ?
|
||
pd.DEFAULT_NPARTITIONS = 4 | ||
|
||
|
||
def generate_dfs(): |
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.
Moving this is fine for this PR, but in the future I prefer new "REFACTOR" PRs for moved code so we can keep a more detailed commit history.
…-project#1684) Signed-off-by: Dmitry Chigarev <dmitry.chigarev@intel.com>
What do these changes do?
Fix of #1683:
pd.concat
losing indices namesflake8 modin
black --check modin
git commit -s