Skip to content
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.

Sign up for GitHub

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

Jump to bottom

FEAT-#7203: Make sure modin works correctly with pandas, which uses pyarrow as a backend #7204

Merged
merged 59 commits into from May 14, 2024
Merged

FEAT-#7203: Make sure modin works correctly with pandas, which uses pyarrow as a backend #7204

merged 59 commits into from May 14, 2024

Conversation

anmyachev
Copy link
Collaborator

@anmyachev anmyachev commented Apr 19, 2024 •
edited

What do these changes do?

Blocked by #7199

The main idea is to disable type precomputation for a dataframe created using pyarrow backend for pandas. For this purpose, additional information obtained when creating internal PandasDataframe is used. Dataframes created from such a dataframe inherit this information. Enabling type precomputation occurs after the types are materialized, if the backend is default. This can also happen when calling astype function. This solution is not the fastest in terms of performance, but it is the most reliable and easiest to support.

  • first commit message and PR title follow format outlined here

    NOTE: If you edit the PR title to match this format, you need to add another commit (even if it's empty) or amend your last commit for the CI job that checks the PR title to pick up the new PR title.

  • passes flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
  • passes black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
  • signed commit with git commit -s
  • Resolves Modin should work correctly with pandas, which uses pyarrow as a backend #7203
  • tests added and passing
  • module layout described at docs/development/architecture.rst is up-to-date

@anmyachev
TEST-modin-project#7049: Add some sanity tests with pyarrow-backed pa…
8e46e4e 
…ndas dataframes

Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
fixes
6814c6e 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
fix
e1dbc69 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
Merge branch 'main' of https://github.com/modin-project/modin into is…
0241d7f 
…sue7049
@anmyachev
cleanup
7b925a5 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
fix comment
23003c5 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
skip some cases for HDK
cc2a5ab 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
FEAT-modin-project#7203: Make sure modin works correctly with pandas,…
b710865 
… which uses pyarrow as a backend

Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
don't use numpy types directly
310f12a 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
try another dtype_backend
cb90479 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
fix
f9b2560 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
Merge branch 'main' of https://github.com/modin-project/modin into is…
907fc9a 
…sue7203
@anmyachev
Merge branch 'main' of https://github.com/modin-project/modin into is…
13e0d0e 
…sue7203
@anmyachev
fixes
ddcda4f 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
fix
afae62f 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
fix
316cddb 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
fix pivot_table
639c2ed 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
fix
05f32e5 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
find potential problem areas at the query_compiler level
194cc68 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
some more places
91f2607 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
add construct_dtype
e24201f 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
fix
4dba613 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
Merge branch 'main' of https://github.com/modin-project/modin into is…
5f56c4a 
…sue7203
@anmyachev
fix
ea05389 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
fix
005f480 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
fix
0d34bea 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev
Copy link
Collaborator Author

anmyachev commented May 14, 2024 •
edited

Blocked on #7264.

@anmyachev anmyachev marked this pull request as ready for review May 14, 2024 09:13
anmyachev
anmyachev commented May 14, 2024
View reviewed changes
modin/core/dataframe/pandas/metadata/dtypes.py
# meaning that we shouldn't try computing a new dtype for this column,
# so marking it as 'unknown'
i: (
pandas.api.types.pandas_dtype(float)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC there is no point in filling nans with this value, since this will be done later in the code.

@anmyachev
cleanup
b3471ff 
Signed-off-by: Anatoly Myachev <[email protected]>
anmyachev and others added 2 commits May 14, 2024 15:54
YarShev
YarShev reviewed May 14, 2024
View reviewed changes
modin/tests/pandas/dataframe/test_binary.py Outdated Show resolved Hide resolved
modin/pandas/utils.py Show resolved Hide resolved
modin/core/storage_formats/base/query_compiler.py Outdated Show resolved Hide resolved
modin/pandas/dataframe.py Outdated Show resolved Hide resolved
modin/pandas/dataframe.py Outdated Show resolved Hide resolved
modin/core/storage_formats/pandas/query_compiler.py Show resolved Hide resolved
modin/core/storage_formats/pandas/query_compiler.py Show resolved Hide resolved
modin/core/storage_formats/pandas/groupby.py Show resolved Hide resolved
modin/core/dataframe/pandas/dataframe/dataframe.py Show resolved Hide resolved
modin/core/dataframe/pandas/dataframe/dataframe.py Show resolved Hide resolved
@anmyachev
address review comments
3213194 
Signed-off-by: Anatoly Myachev <[email protected]>
YarShev
YarShev previously approved these changes May 14, 2024
View reviewed changes
Copy link
Collaborator

@YarShev YarShev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Left some minor comments.

@anmyachev
expand comments
45acef9 
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev anmyachev merged commit aa6e5c2 into modin-project:main May 14, 2024
54 checks passed
@anmyachev anmyachev deleted the issue7203 branch May 14, 2024 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YarShev YarShev YarShev approved these changes

@devin-petersohn devin-petersohn Awaiting requested review from devin-petersohn devin-petersohn is a code owner

@mvashishtha mvashishtha Awaiting requested review from mvashishtha mvashishtha is a code owner

@RehanSD RehanSD Awaiting requested review from RehanSD RehanSD is a code owner

@vnlitvinov vnlitvinov Awaiting requested review from vnlitvinov vnlitvinov is a code owner

@dchigarev dchigarev Awaiting requested review from dchigarev dchigarev is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Modin should work correctly with pandas, which uses pyarrow as a backend
2 participants
@anmyachev @YarShev