pandas/conftest.py refactoring

[SaturnFromTitan]

As stated in this review comment from jreback, we should address some refactoring issues in pandas/conftest.py:

• [x] create the indices fixture using tm.all_index_generator as suggested by jbrockmendel in this comment
• [x] all fixtures and functions should have a docstring
• [x] add sections, i.e. "some text markers so reading the file is easy"
• [ ] remove/avoid imports from conftest.py

[SaturnFromTitan]

If somebody wants to look into any of these, please feel free to do so - ideally with one PR per issue.

If no one does so, I'll probably take it in the coming 2 weeks.

[jorisvandenbossche]

remove/avoid imports from conftest.py

What do you mean with this?

[simonjayhawkins]

remove/avoid imports from conftest.py

we could consider adding a code check rule to enforce this. IIUC pytest already prohibits importing fixtures, but currently nothing stops us importing variables.

in general, when doing this, it's probably an indicator that the fixtures are not used correctly (or in some cases maybe not designed appropriately)

[jorisvandenbossche]

We also define variables (constants) in conftest.py that are used in tests, and those are not always fixtures and thus need to be imported.

BTW, importing from conftest.py is much easier to understand/find and more explicit than the magic of pytest.

[jreback]

We also define variables (constants) in conftest.py that are used in tests, and those are not always fixtures and thus need to be imported.

BTW, importing from conftest.py is much easier to understand/find and more explicit than the magic of pytest.

we should never import from conftest for any reason

it is non idiomatic ; and very arbitrary
everything is a fixture

[jorisvandenbossche]

everything is a fixture

You maybe want that, but it's not the reality ;)

[jreback]

everything is a fixture

You maybe want that, but it's not the reality ;)

exactly so why exacerbate it - we need a path to consistency - this is it

[jbrockmendel]

BTW, importing from conftest.py is much easier to understand/find and more explicit than the magic of pytest.

+1 for not-magic.

If there's something we need in conftest and want to be able to import, let's just put it in pandas._testing or something

[jorisvandenbossche]

Moving things out of conftest we want to import is fine for me as well (although pandas._testing is already a huge file, so maybe we should make it a module)

[MarcoGorelli]

we could consider adding a code check rule to enforce this

xref #30914 :)

[SaturnFromTitan]

Looking into this right now and pondering on

add sections, i.e. "some text markers so reading the file is easy"

Adding such text markers is always an indicator for too large files. Instead, I suggest to create a directory _conftest with files:

• conftest_dtypes.py
• conftest_autouse.py
• ...
• __init__.py (* importing everything from the other files)

and then use a simple from pandas._conftest import * in pandas/conftest.py.

What do you think @jreback @simonjayhawkins @jorisvandenbossche?

[jorisvandenbossche]

I am slightly -1 on splitting conftest.py for now. It's already magical how fixtures get used and thus hard to know where they are defined. Splitting makes that even harder IMO (now, at some point it probably gets unavoidable, but right now I am not sure conftest.py is already annoyingly long)

[simonjayhawkins]

agree with @jorisvandenbossche for now. adding sections to conftest.py to address https://github.com/pandas-dev/pandas/pull/31701#discussion_r379705368 may help make the case for splitting in the future

[jreback]

+1 in adding commented sections but agree split not needed yet

[SaturnFromTitan]

Closing this. I'll handle the remaining task "remove/avoid imports from conftest.py" in #30914