TST: use env variable instead of pytest option for testing ArrayManager

[jorisvandenbossche]

I noticed a problem with the current --array-manager pytest option to test the ArrayManager, while checking the window tests.

The problem with the option is that conftest.py files are read first, before handling the option in pytest_sessionstart. This means that any DataFrame or Series that gets constructed on import of a conftest.py file, has already been created before the setting is changed, and will thus be created with the wrong manager.
Example of this is a DataFrame that is not created inside a (fixture) function, but created top-level in the conftest.py file (or eg used in parametrization).

Luckily we don't have too many of those, but one example in window/conftest.py:

pandas/pandas/tests/window/conftest.py

Lines 352 to 367 in 74a6898

	@pytest.fixture(
	params=[
	DataFrame([[2, 4], [1, 2], [5, 2], [8, 1]], columns=[1, 0]),
	DataFrame([[2, 4], [1, 2], [5, 2], [8, 1]], columns=[1, 1]),
	DataFrame([[2, 4], [1, 2], [5, 2], [8, 1]], columns=["C", "C"]),
	DataFrame([[2, 4], [1, 2], [5, 2], [8, 1]], columns=[1.0, 0]),
	DataFrame([[2, 4], [1, 2], [5, 2], [8, 1]], columns=[0.0, 1]),
	DataFrame([[2, 4], [1, 2], [5, 2], [8, 1]], columns=["C", 1]),
	DataFrame([[2.0, 4.0], [1.0, 2.0], [5.0, 2.0], [8.0, 1.0]], columns=[1, 0.0]),
	DataFrame([[2, 4.0], [1, 2.0], [5, 2.0], [8, 1.0]], columns=[0, 1.0]),
	DataFrame([[2, 4], [1, 2], [5, 2], [8, 1.0]], columns=[1.0, "X"]),
	]
	)
	def pairwise_frames(request):
	"""Pairwise frames test_pairwise"""
	return request.param

or one example in the toplevel conftest.py:

pandas/pandas/conftest.py

Lines 597 to 608 in 74a6898

	def _create_series(index):
	""" Helper for the _series dict """
	size = len(index)
	data = np.random.randn(size)
	return pd.Series(data, index=index, name="a")
	_series = {
	f"series-with-{index_id}-index": _create_series(index)
	for index_id, index in indices_dict.items()
	}

So to avoid needing to rewrite those (and continue to ensure such cases don't get added), I don't see another way as using the environment variable approach instead of the pytest option (although the option is much easier to use interactively ..)

cc @jreback

[jreback]

small comment, looks fine to me

[jreback]

i would just inline this on L494 (and add a 1-liner comment before) and/or update the option description with the text on how to control this

[jreback]

Luckily we don't have too many of those, but one example in window/conftest.py:

should we just change / check for these?

[jorisvandenbossche]

should we just change / check for these?

I think that's a bit fragile to ensure that in the future (and in theory there is also nothing wrong with it, just not working with this option that influences the dataframe creation). Would there be an easy way to check this?

[jreback]

should we just change / check for these?

I think that's a bit fragile to ensure that in the future (and in theory there is also nothing wrong with it, just not working with this option that influences the dataframe creation). Would there be an easy way to check this?

agree its fragile, but would be good to check it, yeah not sure its easy.

[jreback]

lgtm

[jorisvandenbossche]

If someone comes up with a good approach for keeping it with the pytest option, we can always change back. But for now I want to ensure I am correctly testing ArrayManager ;)