Refactor#3622 #3645

teaVeloper · 2021-11-02T18:25:07Z

What do these changes do?

Working on Issue #3622
Move Tests for modin.pandas into directory modin/test/pandas and changing all references either in

imports
conftest.py
file readers
documentation
Github Actions

not resolving issue completely, as same needs to be done for other tests (I will do, after this PR is accepted, or shall I work on the whole issue before openin PR?).

commit message follows format outlined here
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 Centralize tests in Modin #3622
tests added and passing
module layout described at docs/developer/architecture.rst is up-to-date

codecov · 2021-11-02T18:51:52Z

Codecov Report

Merging #3645 (bd959b5) into master (99e8507) will decrease coverage by 21.71%.
The diff coverage is 66.66%.

@@             Coverage Diff             @@
##           master    #3645       +/-   ##
===========================================
- Coverage   85.69%   63.98%   -21.72%     
===========================================
  Files         201      196        -5     
  Lines       16866    16667      -199     
===========================================
- Hits        14454    10664     -3790     
- Misses       2412     6003     +3591

Impacted Files	Coverage Δ
modin/conftest.py	`75.30% <66.66%> (-7.00%)`	⬇️
modin/distributed/__init__.py	`0.00% <0.00%> (-100.00%)`	⬇️
modin/experimental/core/__init__.py	`0.00% <0.00%> (-100.00%)`	⬇️
modin/core/execution/ray/__init__.py	`0.00% <0.00%> (-100.00%)`	⬇️
modin/experimental/pandas/__init__.py	`0.00% <0.00%> (-100.00%)`	⬇️
modin/experimental/sklearn/__init__.py	`0.00% <0.00%> (-100.00%)`	⬇️
modin/experimental/xgboost/__init__.py	`0.00% <0.00%> (-100.00%)`	⬇️
modin/distributed/dataframe/__init__.py	`0.00% <0.00%> (-100.00%)`	⬇️
modin/experimental/core/execution/__init__.py	`0.00% <0.00%> (-100.00%)`	⬇️
modin/distributed/dataframe/pandas/__init__.py	`0.00% <0.00%> (-100.00%)`	⬇️
... and 76 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99e8507...bd959b5. Read the comment docs.

modin-bot · 2021-11-02T18:56:51Z

TeamCity Dask test results bot

Tests PASSed

Tests Logs

============================= test session starts ==============================
platform linux -- Python 3.8.12, pytest-7.0.0, pluggy-1.0.0
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /modin, configfile: setup.cfg
plugins: benchmark-3.4.1, cov-2.11.0, forked-1.4.0, xdist-2.5.0
collected 0 items

============================ no tests ran in 0.03s =============================
ERROR: file or directory not found: modin/pandas/test/test_io.py

============================= test session starts ==============================
platform linux -- Python 3.8.12, pytest-7.0.0, pluggy-1.0.0
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /modin, configfile: setup.cfg
plugins: benchmark-3.4.1, cov-2.11.0, forked-1.4.0, xdist-2.5.0
collected 0 items

============================ no tests ran in 0.03s =============================
ERROR: file or directory not found: modin/pandas/test/test_io.py

modin-bot · 2021-11-02T18:57:23Z

TeamCity Python test results bot

Tests PASSed

Tests Logs

============================= test session starts ==============================
platform linux -- Python 3.8.12, pytest-7.0.0, pluggy-1.0.0
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /modin, configfile: setup.cfg
plugins: benchmark-3.4.1, cov-2.11.0, forked-1.4.0, xdist-2.5.0
collected 0 items

============================ no tests ran in 0.03s =============================
ERROR: file or directory not found: modin/pandas/test/test_io.py

============================= test session starts ==============================
platform linux -- Python 3.8.12, pytest-7.0.0, pluggy-1.0.0
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /modin, configfile: setup.cfg
plugins: benchmark-3.4.1, cov-2.11.0, forked-1.4.0, xdist-2.5.0
collected 0 items

============================ no tests ran in 0.03s =============================
ERROR: file or directory not found: modin/pandas/test/test_io.py

modin-bot · 2021-11-02T19:03:14Z

TeamCity Ray test results bot

Tests PASSed

Tests Logs

============================= test session starts ==============================
platform linux -- Python 3.8.12, pytest-7.0.0, pluggy-1.0.0
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /modin, configfile: setup.cfg
plugins: benchmark-3.4.1, cov-2.11.0, forked-1.4.0, xdist-2.5.0
collected 0 items

============================ no tests ran in 0.03s =============================
ERROR: file or directory not found: modin/pandas/test/test_io.py

============================= test session starts ==============================
platform linux -- Python 3.8.12, pytest-7.0.0, pluggy-1.0.0
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /modin, configfile: setup.cfg
plugins: benchmark-3.4.1, cov-2.11.0, forked-1.4.0, xdist-2.5.0
collected 0 items

============================ no tests ran in 0.03s =============================
ERROR: file or directory not found: modin/pandas/test/test_io.py

devin-petersohn

@teaVeloper The structure looks great. We have some flaky tests at the moment, so I am rerunning CI to make sure everything is working as intended.

@YarShev can we get TeamCity updated to reflect this new test structure?

YarShev · 2021-11-18T13:15:36Z

@YarShev can we get TeamCity updated to reflect this new test structure?

Sure, but first we will need to be agreed on the changes. As soon as we are okay with the changes, we will update TeamCity to run tests. If tests pass in TeamCity too, we'll merge the PR. Does that makes sense?

YarShev · 2021-11-18T13:19:34Z

.github/workflows/ci.yml

@@ -147,7 +147,7 @@ jobs:
          conda list
      - run: sudo apt update && sudo apt install -y libhdf5-dev
      - name: Api tests
-        run: python -m pytest modin/pandas/test/test_api.py
+        run: python -m pytest modin/test/pandas/test_api.py


Do we want to put all test directories in one place (modin/config/test, modin/core/execution/dispatching/factories/test, modin/experimental/core/execution/native/implementations/omnisci_on_native/test, etc.)?

@YarShev I think that will be best, yes.

Hi, so i guess I should continue refactoring with all folders (i wanted to check if its fine with that one folder, but I guess thats not the best workflow, especially for this issue) and after I will update the PR and then it makes sense to check for the modification with the CI and test pipeline?

@teaVeloper Yes, I think that would be good. It looks great so far!

YarShev · 2022-01-20T07:33:53Z

@teaVeloper, @devin-petersohn, what is the status here?

teaVeloper · 2022-01-20T21:18:40Z

@teaVeloper, @devin-petersohn, what is the status here?

@YarShev
Im sorry for having been silent that long, as I have been pretty busy. I will be able to get all done in the next couple days and update the pull request then regarding.

YarShev · 2022-01-21T08:12:44Z

@teaVeloper, great, thanks! Feel free to ask any questions.

lgtm-com · 2022-01-30T20:53:37Z

This pull request fixes 1 alert when merging a9b72330ce75fa69f02ef0d97e01687e52c86a7d into 035f502 - view on LGTM.com

fixed alerts:

1 for Wrong name for an argument in a class instantiation

devin-petersohn · 2022-01-31T16:40:24Z

@teaVeloper thanks so much for the work on this so far!

The conflicts look pretty complicated. @RehanSD led a major refactor, do you mind if he takes a look and tries to help resolve the conflicts here?

teaVeloper · 2022-01-31T21:31:43Z

Hi @devin-petersohn thanks for feedback.

Sure I am happy to get assistance by @RehanSD with this issues!
Also I did rebase on origin/master before continuing my edits, I hope though I did not mess anything up with resolving the conflicts locally.

RehanSD · 2022-01-31T21:57:45Z

Hi @teaVeloper! Just wanted to check in to see how best we could collaborate! We could either use the Modin slack to pair program and resolve the conflicts, or if you'd prefer, I can always resolve them locally (on my laptop) on your branch, and push to this PR! Please let me know which one you'd prefer! Looking forward to collaborating!

teaVeloper · 2022-02-01T13:20:21Z

Hi @RehanSD thanks for reaching out! I would prefer to pair program and using slack (but I need an invitation I guess?) if that's fine for you? Ill send an email to your berkeley.edu mail address for detailed communication.

Move all modin.pandas releated tests from modin/modin/pandas/test into modin/modin/test/pandas. change conftest.py to adapt regarding. Change entries in CI - .github/workflows/*.yml to use the changed folders instead of the old ones Signed-off-by: Bertold Sedlak <berti.sedlak@gmail.com>

Fix import error within experimental pandas test using previously moved test file Signed-off-by: Bertold Sedlak <berti.sedlak@gmail.com>

Fix references in Files to import modin.pandas.test or using modin/pandas/test to new version Also fix references in documentation. Signed-off-by: Bertold Sedlak <berti.sedlak@gmail.com>

Move all tests from modin/config/test to modin/test/config replace reference in setup.cfg

Move contents modin/core/execution/dispatching/factories/test/ to modin/test/core/execution/dispatching/factories/ as well as entry in setup.cfg

Move modin/experimental/cloud/test/* to modin/test/experimental/cloud/* and entry in setup.cfg

Move all test folders from within experimental to modin/test/experimental also update entries in setup.cfg regarding

Chgange all references to tests to new folder structure

Change all test references to new folder structure

Change all references in github workflows push.yml to new folder structure

Move all modin.pandas releated tests from modin/modin/pandas/test into modin/modin/test/pandas. change conftest.py to adapt regarding. Change entries in CI - .github/workflows/*.yml to use the changed folders instead of the old ones Signed-off-by: Bertold Sedlak <berti.sedlak@gmail.com>

Run black and flake8 resolve forgotten merge conflict

lgtm-com · 2022-02-07T18:17:09Z

This pull request fixes 1 alert when merging bd959b5 into 99e8507 - view on LGTM.com

fixed alerts:

1 for Wrong name for an argument in a class instantiation

YarShev · 2023-12-05T11:53:22Z

@RehanSD, @teaVeloper, what is the status here?

RehanSD · 2023-12-07T18:13:23Z

Hi @YarShev! I think that this PR is stale - unless @teaVeloper has interest in taking it up again, I'd recommend closing for now, and attempting a refactor in a new PR?

YarShev · 2023-12-08T09:03:40Z

@teaVeloper, I am closing this PR as stale. Please reopen it if needed or create a new one if you feel to continue work on this.

teaVeloper requested a review from a team as a code owner November 2, 2021 18:25

devin-petersohn reviewed Nov 8, 2021

View reviewed changes

YarShev reviewed Nov 18, 2021

View reviewed changes

teaVeloper requested a review from a team as a code owner January 30, 2022 20:20

teaVeloper added 13 commits February 7, 2022 17:23

REFACTOR-#3622: Fix import Error

c810138

Fix import error within experimental pandas test using previously moved test file Signed-off-by: Bertold Sedlak <berti.sedlak@gmail.com>

REFACTOR-#3622: Fix references to pandas*test

b2b3376

Fix references in Files to import modin.pandas.test or using modin/pandas/test to new version Also fix references in documentation. Signed-off-by: Bertold Sedlak <berti.sedlak@gmail.com>

Move of config test

098a60a

Move all tests from modin/config/test to modin/test/config replace reference in setup.cfg

Move of core/execution/dispatching/factories/test

1e3e1bd

Move contents modin/core/execution/dispatching/factories/test/ to modin/test/core/execution/dispatching/factories/ as well as entry in setup.cfg

Move new pandas/test to test/pandas

79e86ae

Move tests modin/experimental/cloud

7f94acd

Move modin/experimental/cloud/test/* to modin/test/experimental/cloud/* and entry in setup.cfg

Move remaining tests from experimental

fafa868

Move all test folders from within experimental to modin/test/experimental also update entries in setup.cfg regarding

Change test entries in ci.yaml

ff896fb

Chgange all references to tests to new folder structure

Change test references in push-to-master.yml

2991990

Change all test references to new folder structure

Change test references in push.yml

4f35c0d

Change all references in github workflows push.yml to new folder structure

Run Lint

d9dfc1e

Run black and flake8 resolve forgotten merge conflict

Run black in newest version

bd959b5

YarShev mentioned this pull request Feb 23, 2022

FEAT-#4245: Define base interface for dataframe exchange protocol #4246

Merged

8 tasks

YarShev closed this Dec 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor#3622 #3645

Refactor#3622 #3645

teaVeloper commented Nov 2, 2021 •

edited by YarShev

Loading

codecov bot commented Nov 2, 2021 •

edited

Loading

modin-bot commented Nov 2, 2021 •

edited

Loading

modin-bot commented Nov 2, 2021 •

edited

Loading

modin-bot commented Nov 2, 2021 •

edited

Loading

devin-petersohn left a comment

YarShev commented Nov 18, 2021

YarShev Nov 18, 2021

devin-petersohn Nov 18, 2021

teaVeloper Nov 18, 2021

devin-petersohn Nov 29, 2021

YarShev commented Jan 20, 2022

teaVeloper commented Jan 20, 2022

YarShev commented Jan 21, 2022

lgtm-com bot commented Jan 30, 2022

devin-petersohn commented Jan 31, 2022

teaVeloper commented Jan 31, 2022

RehanSD commented Jan 31, 2022

teaVeloper commented Feb 1, 2022

lgtm-com bot commented Feb 7, 2022

YarShev commented Dec 5, 2023

RehanSD commented Dec 7, 2023

YarShev commented Dec 8, 2023

Refactor#3622 #3645

Refactor#3622 #3645

Conversation

teaVeloper commented Nov 2, 2021 • edited by YarShev Loading

What do these changes do?

codecov bot commented Nov 2, 2021 • edited Loading

Codecov Report

modin-bot commented Nov 2, 2021 • edited Loading

TeamCity Dask test results bot

Tests PASSed

modin-bot commented Nov 2, 2021 • edited Loading

TeamCity Python test results bot

Tests PASSed

modin-bot commented Nov 2, 2021 • edited Loading

TeamCity Ray test results bot

Tests PASSed

devin-petersohn left a comment

Choose a reason for hiding this comment

YarShev commented Nov 18, 2021

YarShev Nov 18, 2021

Choose a reason for hiding this comment

devin-petersohn Nov 18, 2021

Choose a reason for hiding this comment

teaVeloper Nov 18, 2021

Choose a reason for hiding this comment

devin-petersohn Nov 29, 2021

Choose a reason for hiding this comment

YarShev commented Jan 20, 2022

teaVeloper commented Jan 20, 2022

YarShev commented Jan 21, 2022

lgtm-com bot commented Jan 30, 2022

devin-petersohn commented Jan 31, 2022

teaVeloper commented Jan 31, 2022

RehanSD commented Jan 31, 2022

teaVeloper commented Feb 1, 2022

lgtm-com bot commented Feb 7, 2022

YarShev commented Dec 5, 2023

RehanSD commented Dec 7, 2023

YarShev commented Dec 8, 2023

teaVeloper commented Nov 2, 2021 •

edited by YarShev

Loading

codecov bot commented Nov 2, 2021 •

edited

Loading

modin-bot commented Nov 2, 2021 •

edited

Loading

modin-bot commented Nov 2, 2021 •

edited

Loading

modin-bot commented Nov 2, 2021 •

edited

Loading