TST: inline empty_frame = DataFrame({}) fixture
State: closed

broken off from #24873

Labels:
name: Refactor
description: Internal refactoring of code
name: Testing
description: pandas testing functions or related to the test suite

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24886?src=pr&el=h1) Report
> Merging [#24886](https://codecov.io/gh/pandas-dev/pandas/pull/24886?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/4b937ff59127bf0edec8e07a667fa222bf2e3b4e?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/24886/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/24886?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master   #24886   +/-   ##
=======================================
  Coverage   92.39%   92.39%           
=======================================
  Files         166      166           
  Lines       52413    52413           
=======================================
  Hits        48425    48425           
  Misses       3988     3988
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.81% <ø> (ø)` | :arrow_up: |
| #single | `42.89% <ø> (ø)` | :arrow_up: |


------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24886?src=pr&el=footer). Last update [4b937ff...cfc2699](https://codecov.io/gh/pandas-dev/pandas/pull/24886?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24886?src=pr&el=h1) Report
> Merging [#24886](https://codecov.io/gh/pandas-dev/pandas/pull/24886?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/42b4c975e55c59da7868f99f9949841aca12a08d?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/24886/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/24886?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master   #24886   +/-   ##
=======================================
  Coverage   91.75%   91.75%           
=======================================
  Files         173      173           
  Lines       52960    52960           
=======================================
  Hits        48595    48595           
  Misses       4365     4365
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.33% <ø> (ø)` | :arrow_up: |
| #single | `41.71% <ø> (ø)` | :arrow_up: |


------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24886?src=pr&el=footer). Last update [42b4c97...6dd2120](https://codecov.io/gh/pandas-dev/pandas/pull/24886?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

@jreback you're moving the goalposts. In #23701 you agreed that singleton fixtures were not a good use case.  In #24769 you argued that fixtures were less verbose and more clear, an argument which can only plausibly refer to non-inlineable fixtures (i.e. not the ones covered by this PR).

> The fixture is well named and obvious. It is so clear that something is testing an empty dataframe.

Even if it weren't ridiculous to claim that `empty_frame` in an arg is somehow more obvious than `DataFrame({})`, then how in the world do you square this with your argument [here](https://github.com/pandas-dev/pandas/pull/24873#issuecomment-456258172) that `Timestamp.now()` is too simple to be a fixture?  Seriously.  Explain to me the relevant distinction between these two cases. 

Freaky Friday body swap remains my main hypothesis.
@jbrockmendel I said exactly

> if it’s truly a singleton the. no a fixture is not warranted

show me a fixture that is a singleton?


> show me a fixture that is a singleton?

Every single one of the ones under discussion returns a single object, i.e. has no params/parameterization.
> Every single one of the ones under discussion returns a single object, i.e. has no params/parameterization.

so why be inconsistent? fixtures are the way to go
functions are simply going to be a mess; we had this once (and still have a fair amount remaining) from the old unittest framework

we have a nice consistent use of fixtures - changing at this stage is a big step backwards
> so why be inconsistent?

So you're agreeing that individually these are useless-at-best but have value because they are consistent with the user pattern of [something]?

This gets us back to the "why not a fixture for `Timestamp.now()`?" question.  To be consistent with these singleton fixtures we would need to make fixtures for essentially every object used anywhere in the tests.
> So you're agreeing that individually these are useless-at-best but have value because they are consistent with the user pattern of [something]?

nope didn’t say or imply that. these fixtures are good because they r consistent. in ANY codebase i would use fixtures like this.

> This gets us back to the "why not a fixture for Timestamp.now()?" question. To be consistent with these singleton fixtures we would need to make fixtures for essentially every object used anywhere in the tests.

actually should probably use a fixture for Timestanp.now.

You are missing the big picture here and the reason I am not going to budge on this at all. We have an extremely large code and test base. It is really hard to be consistent. A lot of things we do we can enforce with linting, but a lot we cannot. 

Fixtures enforce good testing habits by making sure even simple and of course complex things can be tested pretty easily. We have exactly 1 way of creating a frame of a certain class for example (be it empty or floats or mixed or whatever). There is no confusion if empty means no index or no columns or both. In fact the test writer actually doesn’t need to even understand this; we have a well constructed fixture that does it. 

The abstraction helps a lot here. This also promotes readibility (assume the fixtures are well named, then it is immediately obvious what they produce)

Sure you could argue that for simple things functions are enough, but that leads to the inevitable: when should I use a function (over a fixture). Well I think the answer should now be clear; never.

Either construct a test specific thing (which we do quite a lot); if you need something that is generic then use a fixture.


Well at least by saying Timestamp.now() should have a fixture you are being internally consistent.  Is there anything that shouldn’t be a fixture?
I am ok with this one. Can you merge master. Also let's use ``DataFrame()`` and ``Series()`` for the empties.
thanks @jbrockmendel