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

Reduce pytest runtime #10203

Merged
merged 23 commits into from Feb 15, 2022
Merged

Reduce pytest runtime #10203

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
4d7ae60
address first few slowest tests
brandon-b-miller Feb 3, 2022
1f9e36d
continue reducing runtime
brandon-b-miller Feb 7, 2022
70932d5
updates
brandon-b-miller Feb 7, 2022
b94cce4
??? IDE added this somehow
brandon-b-miller Feb 7, 2022
67e3994
style
brandon-b-miller Feb 7, 2022
e05d8bc
style
brandon-b-miller Feb 7, 2022
f367e01
remove unused parameters
brandon-b-miller Feb 7, 2022
9e37504
Merge branch 'branch-22.04' into fix-reduce-pytest-time
brandon-b-miller Feb 7, 2022
d298d4a
Update python/cudf/cudf/tests/test_binops.py
brandon-b-miller Feb 8, 2022
6e9a0ef
address reviews
brandon-b-miller Feb 8, 2022
e4a98d0
less from_pandas()
brandon-b-miller Feb 8, 2022
289a13b
combine repr tests
brandon-b-miller Feb 8, 2022
2e25ed2
Update python/cudf/cudf/tests/test_indexing.py
brandon-b-miller Feb 8, 2022
9393bd0
update copyrights
brandon-b-miller Feb 8, 2022
a20102e
Apply suggestions from code review
brandon-b-miller Feb 9, 2022
9c800cd
address reviews
brandon-b-miller Feb 9, 2022
025c69d
temporarily revert changes
brandon-b-miller Feb 9, 2022
60a0a92
copyright
brandon-b-miller Feb 9, 2022
6128b2a
Merge branch 'branch-22.04' into fix-reduce-pytest-time
brandon-b-miller Feb 14, 2022
227be3c
redo test_cudf_isclose parameterization
brandon-b-miller Feb 14, 2022
ecdb986
revert changes to test_cudf_isclose
brandon-b-miller Feb 15, 2022
e3b6500
Merge branch 'branch-22.04' into fix-reduce-pytest-time
brandon-b-miller Feb 15, 2022
62d56be
copyright
brandon-b-miller Feb 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions python/cudf/cudf/testing/_utils.py
View file
Open in desktop
@@ -1,5 +1,6 @@
# Copyright (c) 2020-2021, NVIDIA CORPORATION.
Copy link
Contributor

Choose a reason for hiding this comment

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

copyright


import itertools
import re
import warnings
from collections.abc import Mapping, Sequence
Expand Down Expand Up @@ -330,3 +331,9 @@ def does_not_raise():

def xfail_param(param, **kwargs):
return pytest.param(param, marks=pytest.mark.xfail(**kwargs))


numeric_dtypes_pairwise = pytest.mark.parametrize(
brandon-b-miller marked this conversation as resolved.
Show resolved Hide resolved
"left_dtype,right_dtype",
list(itertools.combinations_with_replacement(NUMERIC_TYPES, 2)),
)
2 changes: 1 addition & 1 deletion python/cudf/cudf/tests/test_avro_reader_fastavro_integration.py
View file
Open in desktop
Expand Up @@ -210,7 +210,7 @@ def test_can_parse_no_schema():
assert_eq(expected, actual)
bdice marked this conversation as resolved.
Show resolved Hide resolved


@pytest.mark.parametrize("rows", [0, 1, 10, 100000])
@pytest.mark.parametrize("rows", [0, 1, 10, 1000])
brandon-b-miller marked this conversation as resolved.
Show resolved Hide resolved
@pytest.mark.parametrize("codec", ["null", "deflate", "snappy"])
def test_avro_compression(rows, codec):
schema = {
Expand Down
11 changes: 6 additions & 5 deletions python/cudf/cudf/tests/test_binops.py
View file
Open in desktop
Expand Up @@ -4,7 +4,7 @@
import decimal
import operator
import random
from itertools import product
from itertools import combinations_with_replacement, product

import cupy as cp
import numpy as np
Expand Down Expand Up @@ -217,10 +217,11 @@ def test_series_compare(cmpop, obj_class, dtype):

def _series_compare_nulls_typegen():
tests = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Another way to write this would be:

return [
    *combinations_with_replacement(DATETIME_TYPES, 2),
    *combinations_with_replacement(TIMEDELTA_TYPES, 2),
    *combinations_with_replacement(NUMERIC_TYPES, 2),
    *combinations_with_replacement(STRING_TYPES, 2),
]

However you prefer is fine.

tests += list(product(DATETIME_TYPES, DATETIME_TYPES))
tests += list(product(TIMEDELTA_TYPES, TIMEDELTA_TYPES))
tests += list(product(NUMERIC_TYPES, NUMERIC_TYPES))
tests += list(product(STRING_TYPES, STRING_TYPES))

tests += list(combinations_with_replacement(DATETIME_TYPES, 2))
tests += list(combinations_with_replacement(TIMEDELTA_TYPES, 2))
tests += list(combinations_with_replacement(NUMERIC_TYPES, 2))
tests += list(combinations_with_replacement(STRING_TYPES, 2))

return tests

Expand Down
9 changes: 5 additions & 4 deletions python/cudf/cudf/tests/test_csv.py
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@
from io import BytesIO, StringIO
bdice marked this conversation as resolved.
Show resolved Hide resolved
from pathlib import Path

import cupy as cp
import numpy as np
import pandas as pd
import pytest
Expand Down Expand Up @@ -1009,17 +1010,17 @@ def test_small_zip(tmpdir):
def test_csv_reader_carriage_return(tmpdir):
rows = 1000
names = ["int_row", "int_double_row"]

buffer = ",".join(names) + "\r\n"
for row in range(rows):
buffer += str(row) + ", " + str(2 * row) + "\r\n"

df = read_csv(StringIO(buffer))
expect = cudf.DataFrame(
{"int_row": cp.arange(rows), "int_double_row": cp.arange(rows) * 2}
)

assert len(df) == rows
for row in range(0, rows):
assert df[names[0]][row] == row
assert df[names[1]][row] == 2 * row
assert_eq(expect, df)


def test_csv_reader_tabs():
Expand Down
38 changes: 0 additions & 38 deletions python/cudf/cudf/tests/test_dataframe.py
View file
Open in desktop
Expand Up @@ -6670,7 +6670,6 @@ def test_dataframe_info_null_counts():
"data1",
[
[1, 2, 3, 4, 5, 6, 7],
[1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0],
[
1.9876543,
2.9876654,
Expand All @@ -6689,31 +6688,12 @@ def test_dataframe_info_null_counts():
-6.88918237,
-7.00001,
],
[
1.987654321,
2.987654321,
3.987654321,
0.1221,
2.1221,
0.112121,
-21.1212,
],
[
-1.987654321,
-2.987654321,
-3.987654321,
-0.1221,
-2.1221,
-0.112121,
21.1212,
],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I am just deleting tests that might be redundant. I am basing this off the assumption that all these different sets of numbers are not really increasing test coverage.

Copy link
Contributor

Choose a reason for hiding this comment

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

For some values of rtol and atol, the deleted values seem to be testing the behavior of isclose at a granularity finer than float32 (requiring float64s for accuracy). That seems potentially important, and would deserve a code comment explaining the coverage that each parameter set adds. Instead of deleting these, we might reframe these parameters so that we don't test a whole matrix of all input values and all atol/rtol values (6x6x6x6 = 1296 tests), and only test certain pieces of the matrix that actually cover the important pieces of the behavior (comparing ints and floats, testing isclose at float64 precision, etc.).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAICT the these tests were only covering float64 both before and after the change. The dataframe and array constructors implicitly upcast the incoming data, so it's always getting compared as float64. Is your concern that we're not covering float32 at all here?

Copy link
Contributor

@bdice bdice Feb 11, 2022 •
edited

Choose a reason for hiding this comment

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

Sorry, I wasn't very clear. The issue isn't really about data types. It's about the rtol / atol precision requiring float64 precision, which these changes no longer test adequately. The real-world cases of isclose I have seen use very tight tolerances (sometimes tighter than float32 precision, like 1e-08 for data on the order of 1). Currently, this PR removes the input data that is designed to test those cases of tight tolerances.

If you look at the data1/data2 values like 1.987654321 and 1.9876543, those pairs of input data are meant to be compared with the rtol/atol values of 1e-08 in the other set of parameters. If we remove the more-precise values here, we aren't getting good coverage of the tighter tolerance 1e-08, which requires float64 precision to get the correct results. By removing these pairings of parametrized values, this test would no longer fail if the isclose functions in cuDF or cupy were to erroneously cast their inputs to float32.

I agree that this test is grossly over-parametrized, but the deletions here are removing an important case to check.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's just undo these changes and add a TODO that we can be more clever about parametrizing this particular test. The other changes in this PR give a more-than-meaningful improvement in test time and I don't think it's worth investing much more time over this single test at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that would be good. I can file an issue describing what I've discussed with @brandon-b-miller via Slack.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me. Thank you, Bradley!

Copy link
Contributor

Choose a reason for hiding this comment

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

Issue #10284 filed. I can put this in my backlog of things to do, or I can help someone else construct the specific cases I have in mind for test coverage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change has been reverted

],
)
@pytest.mark.parametrize(
"data2",
[
[1, 2, 3, 4, 5, 6, 7],
[1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0],
[
1.9876543,
2.9876654,
Expand All @@ -6732,24 +6712,6 @@ def test_dataframe_info_null_counts():
-6.88918237,
-7.00001,
],
[
1.987654321,
2.987654321,
3.987654321,
0.1221,
2.1221,
0.112121,
-21.1212,
],
[
-1.987654321,
-2.987654321,
-3.987654321,
-0.1221,
-2.1221,
-0.112121,
21.1212,
],
],
)
@pytest.mark.parametrize("rtol", [0, 0.01, 1e-05, 1e-08, 5e-1, 50.12])
Expand Down
11 changes: 8 additions & 3 deletions python/cudf/cudf/tests/test_extension_compilation.py
View file
Open in desktop
@@ -1,13 +1,16 @@
import operator
bdice marked this conversation as resolved.
Show resolved Hide resolved

import cupy as cp
import numpy as np
import pytest
from numba import cuda, types
from numba.cuda import compile_ptx
from numba.np.numpy_support import from_dtype

from cudf import NA
from cudf.core.udf.api import Masked
from cudf.core.udf.typing import MaskedType
from cudf.testing._utils import numeric_dtypes_pairwise

arith_ops = (
operator.add,
Expand Down Expand Up @@ -159,19 +162,21 @@ def func(x):


@pytest.mark.parametrize("op", ops)
@pytest.mark.parametrize("ty1", number_types, ids=number_ids)
@pytest.mark.parametrize("ty2", number_types, ids=number_ids)
@numeric_dtypes_pairwise
@pytest.mark.parametrize(
"masked",
((False, True), (True, False), (True, True)),
ids=("um", "mu", "mm"),
)
def test_compile_arith_masked_ops(op, ty1, ty2, masked):
def test_compile_arith_masked_ops(op, left_dtype, right_dtype, masked):
def func(x, y):
return op(x, y)

cc = (7, 5)

ty1 = from_dtype(np.dtype(left_dtype))
ty2 = from_dtype(np.dtype(right_dtype))

if masked[0]:
ty1 = MaskedType(ty1)
if masked[1]:
Expand Down
22 changes: 10 additions & 12 deletions python/cudf/cudf/tests/test_indexing.py
View file
Open in desktop
Expand Up @@ -1294,8 +1294,8 @@ def test_loc_datetime_index(sli, is_dataframe):
@pytest.mark.parametrize(
bdice marked this conversation as resolved.
Show resolved Hide resolved
"gdf",
[
cudf.DataFrame({"a": range(1000000)}),
cudf.DataFrame({"a": range(1000000), "b": range(1000000)}),
cudf.DataFrame({"a": range(100000)}),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove the construction of GPU objects from the parametrize call? It occurs at collection time and is very expensive. This can be constructed lazily like:

@pytest.mark.parametrize(
    "gdf_kwargs",
    [
        dict(data={"a": range(100000)}),
        dict(data={"a": range(100000), "b": range(100000)}),
        # ...
        dict(index=[1, 2, 3]),
        # ...
    ],
)

then:

def test_dataframe_sliced(gdf_kwargs, slice):
    gdf = cudf.DataFrame(**gdf_kwargs)
    pdf = gdf.to_pandas()
    # ...

cudf.DataFrame({"a": range(100000), "b": range(100000)}),
cudf.DataFrame({"a": range(20), "b": range(20)}),
brandon-b-miller marked this conversation as resolved.
Show resolved Hide resolved
cudf.DataFrame(
{
Expand All @@ -1305,25 +1305,23 @@ def test_loc_datetime_index(sli, is_dataframe):
}
),
cudf.DataFrame(index=[1, 2, 3]),
cudf.DataFrame(index=range(1000000)),
cudf.DataFrame(index=range(100000)),
cudf.DataFrame(columns=["a", "b", "c", "d"]),
cudf.DataFrame(columns=["a"], index=range(1000000)),
cudf.DataFrame(
columns=["a", "col2", "...col n"], index=range(1000000)
),
cudf.DataFrame(index=cudf.Series(range(1000000)).astype("str")),
cudf.DataFrame(columns=["a"], index=range(100000)),
cudf.DataFrame(columns=["a", "col2", "...col n"], index=range(100000)),
cudf.DataFrame(index=cudf.Series(range(100000)).astype("str")),
cudf.DataFrame(
columns=["a", "b", "c", "d"],
index=cudf.Series(range(1000000)).astype("str"),
index=cudf.Series(range(100000)).astype("str"),
),
],
)
@pytest.mark.parametrize(
"slice",
[
slice(250000, 500000),
slice(250000, 250001),
slice(500000),
slice(25000, 50000),
slice(25000, 25001),
slice(50000),
slice(1, 10),
slice(10, 20),
slice(15, 24000),
Expand Down
24 changes: 13 additions & 11 deletions python/cudf/cudf/tests/test_orc.py
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@

import cudf
from cudf.io.orc import ORCWriter
from cudf.testing import assert_frame_equal
from cudf.testing._utils import (
assert_eq,
gen_rand_series,
Expand Down Expand Up @@ -93,7 +94,7 @@ def test_orc_reader_basic(datadir, inputfile, columns, use_index, engine):
path, engine=engine, columns=columns, use_index=use_index
)

assert_eq(expect, got, check_categorical=False)
assert_frame_equal(cudf.from_pandas(expect), got, check_categorical=False)


def test_orc_reader_filenotfound(tmpdir):
Expand Down Expand Up @@ -384,11 +385,13 @@ def test_orc_writer(datadir, tmpdir, reference_file, columns, compression):
else:
print(type(excpr).__name__)

expect = orcfile.read(columns=columns).to_pandas()
cudf.from_pandas(expect).to_orc(gdf_fname.strpath, compression=compression)
got = pa.orc.ORCFile(gdf_fname).read(columns=columns).to_pandas()
expect = cudf.from_pandas(orcfile.read(columns=columns).to_pandas())
expect.to_orc(gdf_fname.strpath, compression=compression)
got = cudf.from_pandas(
pa.orc.ORCFile(gdf_fname).read(columns=columns).to_pandas()
)

assert_eq(expect, got)
assert_frame_equal(expect, got)


@pytest.mark.parametrize("stats_freq", ["NONE", "STRIPE", "ROWGROUP"])
Expand All @@ -405,11 +408,11 @@ def test_orc_writer_statistics_frequency(datadir, tmpdir, stats_freq):
else:
print(type(excpr).__name__)

expect = orcfile.read().to_pandas()
cudf.from_pandas(expect).to_orc(gdf_fname.strpath, statistics=stats_freq)
got = pa.orc.ORCFile(gdf_fname).read().to_pandas()
expect = cudf.from_pandas(orcfile.read().to_pandas())
expect.to_orc(gdf_fname.strpath, statistics=stats_freq)
got = cudf.from_pandas(pa.orc.ORCFile(gdf_fname).read().to_pandas())

assert_eq(expect, got)
assert_frame_equal(expect, got)


@pytest.mark.parametrize("stats_freq", ["NONE", "STRIPE", "ROWGROUP"])
Expand Down Expand Up @@ -492,8 +495,7 @@ def test_chunked_orc_writer(
writer.close()

got = pa.orc.ORCFile(gdf_fname).read(columns=columns).to_pandas()

assert_eq(expect, got)
assert_frame_equal(cudf.from_pandas(expect), cudf.from_pandas(got))


@pytest.mark.parametrize(
Expand Down
8 changes: 4 additions & 4 deletions python/cudf/cudf/tests/test_parquet.py
View file
Open in desktop
Expand Up @@ -1105,9 +1105,9 @@ def test_parquet_reader_list_large_multi_rowgroup_nulls(tmpdir):
assert_eq(expect, got)


@pytest.mark.parametrize("skip", range(0, 128))
@pytest.mark.parametrize("skip", range(0, 10))
def test_parquet_reader_list_skiprows(skip, tmpdir):
num_rows = 128
num_rows = 10
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I reason that that if 0:10 work then 11:128 should too.

Copy link
Contributor

@bdice bdice Feb 9, 2022 •
edited

Choose a reason for hiding this comment

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

Maybe even replace range(0, 10) with [0, 1, 5, 10]. Maybe even search the tests for the regex parametrize.*range. 🙃

src = pd.DataFrame(
{
"a": list_gen(int_gen, 0, num_rows, 80, 50),
Expand All @@ -1124,9 +1124,9 @@ def test_parquet_reader_list_skiprows(skip, tmpdir):
assert_eq(expect, got, check_dtype=False)


@pytest.mark.parametrize("skip", range(0, 120))
@pytest.mark.parametrize("skip", range(0, 10))
def test_parquet_reader_list_num_rows(skip, tmpdir):
num_rows = 128
num_rows = 20
src = pd.DataFrame(
{
"a": list_gen(int_gen, 0, num_rows, 80, 50),
Expand Down
7 changes: 2 additions & 5 deletions python/cudf/cudf/tests/test_repr.py
View file
Open in desktop
Expand Up @@ -13,7 +13,7 @@
from cudf.testing import _utils as utils
bdice marked this conversation as resolved.
Show resolved Hide resolved
from cudf.utils.dtypes import np_dtypes_to_pandas_dtypes

repr_categories = utils.NUMERIC_TYPES + ["str", "category", "datetime64[ns]"]
repr_categories = ["int64", "float64", "str", "category", "datetime64[ns]"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I reason that we only need one of each kind because every flavor of float or int should have the same __repr__.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here I reason that we only need one of each kind because every flavor of float or int should have the same __repr__.

Agree, can we also add uint16 to cover unsigned int types?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, if the tests can handle bool might try adding it here aswell. Else okay to skip it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added uint. It seems bool was not originally included here, and adding it now creates some failed tests that use this parameterization. We will probably need to update a bunch of them to cover bool. Want me to create an issue around this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, not needed for a new issue if we are covering bool atleast anywhere in our repr testing. Else, we probably want to cover it as part of another PR.



@pytest.mark.parametrize("dtype", repr_categories)
Expand Down Expand Up @@ -85,15 +85,12 @@ def test_full_series(nrows, dtype):


@pytest.mark.parametrize("dtype", repr_categories)
@pytest.mark.parametrize("nrows", [0, 1, 2, 9, 20 / 2, 11, 20 - 1, 20, 20 + 1])
@pytest.mark.parametrize("ncols", [0, 1, 2, 9, 20 / 2, 11, 20 - 1, 20, 20 + 1])
def test_full_dataframe_20(dtype, nrows, ncols):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just merge test_full_dataframe_20 & test_full_dataframe_21 by parametrizing size into one test and also with reduced parametrization of nrows & ncols?

def test_full_dataframe_20(dtype):
size = 20
pdf = pd.DataFrame(
{idx: np.random.randint(0, 100, size) for idx in range(size)}
).astype(dtype)
gdf = cudf.from_pandas(pdf)

assert pdf.__repr__() == gdf.__repr__()
bdice marked this conversation as resolved.
Show resolved Hide resolved
assert pdf._repr_html_() == gdf._repr_html_()
assert pdf._repr_latex_() == gdf._repr_latex_()
Expand Down
6 changes: 3 additions & 3 deletions python/cudf/cudf/tests/test_reshape.py
View file
Open in desktop
Expand Up @@ -17,9 +17,9 @@
)
bdice marked this conversation as resolved.
Show resolved Hide resolved


@pytest.mark.parametrize("num_id_vars", [0, 1, 2, 10])
@pytest.mark.parametrize("num_value_vars", [0, 1, 2, 10])
@pytest.mark.parametrize("num_rows", [1, 2, 1000])
@pytest.mark.parametrize("num_id_vars", [0, 1, 2])
@pytest.mark.parametrize("num_value_vars", [0, 1, 2])
@pytest.mark.parametrize("num_rows", [1, 2, 100])
@pytest.mark.parametrize("dtype", NUMERIC_TYPES + DATETIME_TYPES)
@pytest.mark.parametrize("nulls", ["none", "some", "all"])
def test_melt(nulls, num_id_vars, num_value_vars, num_rows, dtype):
Expand Down
7 changes: 1 addition & 6 deletions python/cudf/cudf/tests/test_string.py
View file
Open in desktop
Expand Up @@ -532,12 +532,7 @@ def _cat_convert_seq_to_cudf(others):
@pytest.mark.parametrize("sep", [None, "", " ", "|", ",", "|||"])
@pytest.mark.parametrize("na_rep", [None, "", "null", "a"])
@pytest.mark.parametrize(
"index",
[
["1", "2", "3", "4", "5"],
pd.Series(["1", "2", "3", "4", "5"]),
pd.Index(["1", "2", "3", "4", "5"]),
],
"index", [["1", "2", "3", "4", "5"]],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAICT the only place the raw index arg is used is to set the index of ps and gs, and then ps.index and gs.index are used after that. But I think all three of the previous parameters result in the same index despite being different objects at the outset. As such I think 2/3 of these test cases are redundant.

)
def test_string_cat(ps_gs, others, sep, na_rep, index):
ps, gs = ps_gs
Expand Down