-
Notifications
You must be signed in to change notification settings - Fork 666
FEAT-#1839: Update pandas dependency and pandas APIs to match #1840
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
FEAT-#1839: Update pandas dependency and pandas APIs to match #1840
Conversation
1cfd7a1
to
835b5d3
Compare
835b5d3
to
1fbe3aa
Compare
9bae5e5
to
5c8f71f
Compare
5c8f71f
to
4d02d08
Compare
Codecov Report
@@ Coverage Diff @@
## master #1840 +/- ##
==========================================
+ Coverage 82.46% 82.48% +0.01%
==========================================
Files 100 100
Lines 10369 10378 +9
==========================================
+ Hits 8551 8560 +9
Misses 1818 1818
Continue to review full report at Codecov.
|
5db060b
to
50578e9
Compare
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.
@YarShev This is looking good, what are the current issues in this branch?
c630706
to
76b339d
Compare
@devin-petersohn , looks like there is some kind of issue with passing parameters to |
f3394d5
to
fcd0aec
Compare
@devin-petersohn , the issue with passing parameters to |
c78715f
to
b3270f7
Compare
The issue with import modin.pandas as pd
import pandas
mdf = pd.DataFrame([[4, 9]] * 3, columns=['A', 'B'])
pdf = pandas.DataFrame([[4, 9]] * 3, columns=['A', 'B'])
mdf.apply(["sum", "sum"])
SpecificationError: Function names must be unique if there is no new column names assigned
pdf.apply(["sum", "sum"])
SpecificationError: Function names must be unique if there is no new column names assigned |
@YarShev It looks like this was an intended change to help stop multiple columns with the same name. Since Modin and pandas match, I think it is okay, we should just make sure in the tests that we check that we throw the same error. |
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.
Should we update to pandas==1.1.2? Changes all look good to me. Since I opened this PR I am not allowed to approve it.
007fa73
to
10b5c4a
Compare
Yes, we can update to pandas==1.1.2. Also, none of issues are arisen and tests are passing. If you don't mind, I can approve the PR and we merge the changes. |
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.
I have a desire to review this:)
e4d5931
to
19e8e96
Compare
19e8e96
to
de4b74d
Compare
… to match Signed-off-by: Devin Petersohn <[email protected]> Signed-off-by: Igoshev, Yaroslav <[email protected]>
de4b74d
to
91f48a8
Compare
After I merged the current master to this branch, two tests became failing:
I suggest fixing these as part of another issues. Regarding to 1 - as part of this issue, regarding to 2 - as part of new issue. What do you think @devin-petersohn , @anmyachev ? |
This is okay with me @YarShev |
Okay, let's wait @anmyachev . If he has no any objections, we will merge the PR. |
about the 1, I don't think it's a part of #2088, that's something happened with |
Of course, I will create an issue for this. We will just link the new issue with #2088. |
Let's merge the PR! |
Signed-off-by: Devin Petersohn [email protected]
What do these changes do?
flake8 modin
black --check modin
git commit -s