TEST-#6994: Update tests in `test_series.py` #6995

anmyachev · 2024-03-04T12:06:27Z

What do these changes do?

Obsolete parameter combinations were found in #6954.

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 Update tests in test_series.py #6994
tests added and passing
module layout described at docs/development/architecture.rst is up-to-date

anmyachev · 2024-03-04T12:08:31Z

modin/pandas/test/test_series.py

        )


-@pytest.mark.parametrize("data", test_data_values, ids=test_data_keys)
-@pytest.mark.parametrize("func", agg_func_values, ids=agg_func_keys)
-def test_aggregate_numeric(request, data, func):


In individual tests that have the word numeric in their names, there is no meaning, since they test a subset of parameters that are already fully tested.

anmyachev · 2024-03-04T12:09:27Z

modin/pandas/test/test_series.py

    eval_general(
        modin_series,
        pandas_series,
-        lambda series: user_warning_checker(
-            series, fn=lambda series: series.aggregate("cumproduct")


cumproduct is deprecated in favour of cumprod.

anmyachev · 2024-03-04T12:11:33Z

modin/pandas/test/test_series.py

-@pytest.mark.parametrize(
-    "skipna", bool_arg_values, ids=arg_keys("skipna", bool_arg_keys)
-)
+@pytest.mark.parametrize("skipna", [False, True])


This is done mainly to avoid testing the None value, since it no longer makes sense (pandas has changed the default value), or pandas throws an exception.

Why should we not be testing this case? Do we throw the same exception?

These values are no longer within the range of acceptable values, and the test coverage has not changed, therefore we do not have specific code for such a case and we can quite freely remove this test case.

I also believe that tests for receiving the same errors for situations where values of an invalid type are passed are not the first priority. So for now I just deleted it.

anmyachev · 2024-03-04T12:12:21Z

modin/pandas/test/test_series.py

 @pytest.mark.parametrize("axis", [None, 0, 1])
-@pytest.mark.parametrize("level", [None, -1, 0, 1])


This argument is no longer used in functions.

anmyachev · 2024-03-04T12:12:51Z

modin/pandas/test/test_series.py

@@ -1842,7 +1724,9 @@ def test_dt(timezone):
    def dt_with_empty_partition(lib):
        # For context, see https://github.com/modin-project/modin/issues/5112
        df = (
-            pd.concat([pd.DataFrame([None]), pd.DataFrame([pd.TimeDelta(1)])], axis=1)


TimeDelta no longer exists.

anmyachev · 2024-03-04T12:16:31Z

modin/pandas/test/test_series.py

@@ -3570,8 +3421,12 @@ def test_tolist(data):


 @pytest.mark.parametrize("data", test_data_values, ids=test_data_keys)
-@pytest.mark.parametrize("func", agg_func_values, ids=agg_func_keys)


Most functions are not classified as transformational; there is little point in testing exceptions in most cases here.

anmyachev · 2024-03-04T12:16:49Z

modin/pandas/test/test_series.py

@@ -4076,7 +3929,7 @@ def test_str_removesuffix(data):


 @pytest.mark.parametrize("data", test_string_data_values, ids=test_string_data_keys)
-@pytest.mark.parametrize("width", int_arg_values, ids=int_arg_keys)
+@pytest.mark.parametrize("width", [-1, 0, 5])


There are an excessive number of parameterization options; we can reduce their number without changing the coverage.

anmyachev · 2024-03-04T12:16:56Z

modin/pandas/test/test_series.py

@@ -4143,7 +3996,7 @@ def test_str_wrap(data, width):
 @pytest.mark.parametrize("data", test_string_data_values, ids=test_string_data_keys)
 @pytest.mark.parametrize("start", int_arg_values, ids=int_arg_keys)
 @pytest.mark.parametrize("stop", int_arg_values, ids=int_arg_keys)
-@pytest.mark.parametrize("step", int_arg_values, ids=int_arg_keys)
+@pytest.mark.parametrize("step", [-2, 1, 3])


There are an excessive number of parameterization options; we can reduce their number without changing the coverage.

anmyachev · 2024-03-04T12:17:02Z

modin/pandas/test/test_series.py

@@ -4358,8 +4211,11 @@ def test_str_rfind(data, sub, start, end):

 @pytest.mark.parametrize("data", test_string_data_values, ids=test_string_data_keys)
 @pytest.mark.parametrize("sub", string_sep_values, ids=string_sep_keys)
-@pytest.mark.parametrize("start", int_arg_values, ids=int_arg_keys)
-@pytest.mark.parametrize("end", int_arg_values, ids=int_arg_keys)
+@pytest.mark.parametrize(


There are an excessive number of parameterization options; we can reduce their number without changing the coverage.

anmyachev · 2024-03-04T12:17:25Z

modin/pandas/test/test_series.py

@@ -4430,14 +4289,6 @@ def test_str_translate(data, pat):
            modin_series, pandas_series, lambda series: series.str.translate(table)
        )

-    # Test delete chars
-    deletechars = "|"


This argument no longer exists.

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>

modin/pandas/test/test_series.py

YarShev · 2024-03-04T13:21:47Z

modin/pandas/test/test_series.py

@@ -3384,22 +3244,16 @@ def test_subtract(data):
    test_data_values + test_data_small_values,
    ids=test_data_keys + test_data_small_keys,
 )
-@pytest.mark.parametrize("axis", axis_values, ids=axis_keys)


Why removed?

It doesn't make sense for the series. I can write a separate test just for this case.

Test added.

Ah, I see. I think we don't even need a test for this😄

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>

YarShev · 2024-03-04T14:50:06Z

modin/pandas/test/test_series.py

+    if func in ("all", "kurt"):
+        func_kwargs["axis"] = axis
+    elif not axis:
+        # FIXME: In these cases, Modin does not throw exceptions like pandas does


Create an issue for this. I am also wondering how it has been passing before?

#7000.

I am also wondering how it has been passing before?

level parameter was used there, and pandas and modin failed with the error that this parameter did not exist. That is, the irrelevant exception was hiding the problem.

modin/pandas/test/test_series.py

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>

YarShev · 2024-03-04T21:43:20Z

modin/pandas/test/test_series.py

        skipna=skipna,
        numeric_only=numeric_only,
        min_count=min_count,
    )


+@pytest.mark.parametrize("operation", ["sum", "shift"])


Maybe remove? I don't think we need to test axis=1 for series since it doesn't make sense.

I would leave it for now, it seems to me that there is still some inconsistency in the processing of this parameter in pandas. Let's make sure we're aligned here with pandas.

anmyachev commented Mar 4, 2024

View reviewed changes

TEST-modin-project#6994: Update tests in test_series.py

d3b8734

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>

anmyachev force-pushed the issue6994 branch from 8231320 to d3b8734 Compare March 4, 2024 12:32

anmyachev marked this pull request as ready for review March 4, 2024 12:33

anmyachev requested review from devin-petersohn, mvashishtha, RehanSD, YarShev, vnlitvinov, dchigarev and a team as code owners March 4, 2024 12:33

YarShev reviewed Mar 4, 2024

View reviewed changes

modin/pandas/test/test_series.py Outdated Show resolved Hide resolved

YarShev reviewed Mar 4, 2024

View reviewed changes

address review comments

89c2b68

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>

YarShev reviewed Mar 4, 2024

View reviewed changes

anmyachev added 2 commits March 4, 2024 17:17

address review comments

3b87e54

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>

fixes

d1b809e

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>

YarShev approved these changes Mar 4, 2024

View reviewed changes

YarShev merged commit 21d10ea into modin-project:master Mar 5, 2024
37 checks passed

anmyachev deleted the issue6994 branch March 5, 2024 09:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TEST-#6994: Update tests in `test_series.py` #6995

TEST-#6994: Update tests in `test_series.py` #6995

anmyachev commented Mar 4, 2024 •

edited

Loading

anmyachev Mar 4, 2024

anmyachev Mar 4, 2024

anmyachev Mar 4, 2024

YarShev Mar 4, 2024

anmyachev Mar 4, 2024

anmyachev Mar 4, 2024

anmyachev Mar 4, 2024

anmyachev Mar 4, 2024

anmyachev Mar 4, 2024

anmyachev Mar 4, 2024

anmyachev Mar 4, 2024

anmyachev Mar 4, 2024

YarShev Mar 4, 2024

anmyachev Mar 4, 2024

anmyachev Mar 4, 2024

YarShev Mar 4, 2024

YarShev Mar 4, 2024

anmyachev Mar 4, 2024

YarShev Mar 4, 2024

anmyachev Mar 4, 2024

		@pytest.mark.parametrize("axis", [None, 0, 1])
		@pytest.mark.parametrize("level", [None, -1, 0, 1])

		@@ -3570,8 +3421,12 @@ def test_tolist(data):


		@pytest.mark.parametrize("data", test_data_values, ids=test_data_keys)
		@pytest.mark.parametrize("func", agg_func_values, ids=agg_func_keys)

TEST-#6994: Update tests in test_series.py #6995

TEST-#6994: Update tests in test_series.py #6995

Conversation

anmyachev commented Mar 4, 2024 • edited Loading

What do these changes do?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TEST-#6994: Update tests in `test_series.py` #6995

TEST-#6994: Update tests in `test_series.py` #6995

anmyachev commented Mar 4, 2024 •

edited

Loading