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

FIX-#5566: Enable test_indexing test on the HDK engine and add to ci #5567

Merged
merged 14 commits into from
Mar 31, 2023
Merged

FIX-#5566: Enable test_indexing test on the HDK engine and add to ci #5567

merged 14 commits into from
Mar 31, 2023

Conversation

AndreyPavlenko
Copy link
Collaborator

@AndreyPavlenko AndreyPavlenko commented Jan 20, 2023 •
edited
Loading

What do these changes do?

Reworked column names encoding.
Fixes.

Depends on #5156

  • 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 Enable test_indexing test on the HDK engine and add to ci #5566
  • tests added and passing
  • module layout described at docs/development/architecture.rst is up-to-date

@AndreyPavlenko AndreyPavlenko requested review from a team as code owners January 20, 2023 15:12
@AndreyPavlenko AndreyPavlenko changed the title Issue 5566 FIX-#5566: Enable test_indexing test on the HDK engine and add to ci Jan 20, 2023
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 20, 2023
View reviewed changes
modin/core/storage_formats/base/query_compiler.py Fixed Show fixed Hide fixed
...mental/core/execution/native/implementations/hdk_on_native/partitioning/partition_manager.py
Comment on lines +115 to +117
def from_arrow(
cls, at, return_dims=False, unsupported_cols=None, encode_col_names=True
):

Check notice

Code scanning / CodeQL

Returning tuples with varying lengths

HdkOnNativeDataframePartitionManager.from_arrow returns [tuple of size 2](1) and [tuple of size 4](2).
@AndreyPavlenko AndreyPavlenko force-pushed the issue-5566 branch 3 times, most recently from a285bdb to 7f61b0c Compare January 20, 2023 22:34
@AndreyPavlenko AndreyPavlenko force-pushed the issue-5566 branch 3 times, most recently from b829864 to 7038538 Compare February 7, 2023 21:31
anmyachev
anmyachev reviewed Feb 21, 2023
View reviewed changes
modin/pandas/test/utils.py Outdated Show resolved Hide resolved
modin/pandas/test/dataframe/test_indexing.py Outdated Show resolved Hide resolved
modin/pandas/test/dataframe/test_indexing.py Outdated Show resolved Hide resolved
modin/pandas/test/dataframe/test_indexing.py Outdated
Comment on lines 60 to 61
# Initialize env for storage format detection in @pytest.mark.xfail
pd.DataFrame()
Copy link
Collaborator

Choose a reason for hiding this comment

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

If it is really necessary, then it will probably need to be moved to conftest.py file.

modin/pandas/dataframe.py Outdated
@@ -1212,6 +1212,7 @@ def insert(
if (
is_list_like(value)
and not isinstance(value, (pandas.Series, Series))
and len(self.index) != 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why?

modin/experimental/core/storage_formats/hdk/query_compiler.py Outdated Show resolved Hide resolved
modin/experimental/core/storage_formats/hdk/query_compiler.py Outdated Show resolved Hide resolved
@AndreyPavlenko AndreyPavlenko force-pushed the issue-5566 branch 3 times, most recently from 02bf562 to 298911e Compare March 15, 2023 23:45
@AndreyPavlenko AndreyPavlenko force-pushed the issue-5566 branch 4 times, most recently from d6050ed to b8d7b2c Compare March 21, 2023 01:15
@AndreyPavlenko AndreyPavlenko force-pushed the issue-5566 branch 2 times, most recently from f0d0a94 to b90d408 Compare March 22, 2023 12:28
anmyachev
anmyachev reviewed Mar 22, 2023
View reviewed changes
requirements/env_hdk.yml Outdated Show resolved Hide resolved
@anmyachev
Copy link
Collaborator

@AndreyPavlenko HDK tests are failed.

dchigarev
dchigarev previously approved these changes Mar 31, 2023
View reviewed changes
anmyachev
anmyachev previously approved these changes Mar 31, 2023
View reviewed changes
YarShev
YarShev reviewed Mar 31, 2023
View reviewed changes
modin/experimental/core/execution/native/implementations/hdk_on_native/dataframe/utils.py Show resolved Hide resolved
modin/pandas/test/dataframe/test_indexing.py Outdated Show resolved Hide resolved
modin/experimental/core/execution/native/implementations/hdk_on_native/test/test_utils.py Outdated Show resolved Hide resolved
...mental/core/execution/native/implementations/hdk_on_native/partitioning/partition_manager.py Show resolved Hide resolved
modin/experimental/core/execution/native/implementations/hdk_on_native/dataframe/utils.py Show resolved Hide resolved
modin/experimental/core/execution/native/implementations/hdk_on_native/dataframe/utils.py Show resolved Hide resolved
modin/experimental/core/execution/native/implementations/hdk_on_native/dataframe/utils.py Show resolved Hide resolved
@AndreyPavlenko AndreyPavlenko dismissed stale reviews from anmyachev and dchigarev via a934467 March 31, 2023 15:29
AndreyPavlenko and others added 14 commits March 31, 2023 17:34
@AndreyPavlenko
FIX-modin-project#5566: Enable test_indexing test on the HDK engine a…
6702440 
…nd add to ci

Signed-off-by: Andrey Pavlenko <andrey.a.pavlenko@gmail.com>
@AndreyPavlenko
Improved performance of the col name encoding
11abd83
@AndreyPavlenko @anmyachev
Apply suggestions from code review
b5b751d 
Co-authored-by: Anatoly Myachev <anatoliimyachev@mail.com>
@AndreyPavlenko
Fixed test failure caused by FIX-modin-project#5737
e2ceeba
@AndreyPavlenko
Enabled tests skipped previously due to _propagate_index_objs
6458202
@AndreyPavlenko
Added test_utils to ci
4b9db42
@AndreyPavlenko @anmyachev
Apply suggestions from code review
b333e62 
Co-authored-by: Anatoly Myachev <anatoliimyachev@mail.com>
@AndreyPavlenko
Fixed test failures
0330eff
@AndreyPavlenko @anmyachev
Apply suggestions from code review
9569d00 
Co-authored-by: Anatoly Myachev <anatoliimyachev@mail.com>
@AndreyPavlenko
Use LRU cache for col name encoding/decoding
b5522c9
@AndreyPavlenko
Reworked column name encoding
76ffe47
@AndreyPavlenko @dchigarev
Apply suggestions from code review
855a42a 
Co-authored-by: Dmitry Chigarev <dmitry.chigarev@intel.com>
@AndreyPavlenko
Apply suggestions from code review
2ed811d
@AndreyPavlenko @YarShev
Apply suggestions from code review
c27b4f1 
Co-authored-by: Iaroslav Igoshev <Poolliver868@mail.ru>
@Garra1980
Copy link
Collaborator

Let's merge then?

@anmyachev
Copy link
Collaborator

Let's merge then?

@Garra1980 If we are not in a hurry, then it is better to wait for the end of testing.

@Garra1980
Copy link
Collaborator

Let's merge then?

@Garra1980 If we are not in a hurry, then it is better to wait for the end of testing.

Absolutely - just thought testing is ok already

@YarShev YarShev merged commit cffce29 into modin-project:master Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anmyachev anmyachev anmyachev left review comments

@YarShev YarShev YarShev approved these changes

@dchigarev dchigarev dchigarev approved these changes

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

Successfully merging this pull request may close these issues.

Enable test_indexing test on the HDK engine and add to ci
5 participants
@AndreyPavlenko @anmyachev @dchigarev @Garra1980 @YarShev