TST: inline empty_frame = DataFrame({}) fixture

[jbrockmendel]

broken off from #24873

[codecov]

Codecov Report

Merging #24886 into master will not change coverage.
The diff coverage is n/a.

@@           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% <ø> (ø)	⬆️
#single	42.89% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b937ff...cfc2699. Read the comment docs.

[codecov]

Codecov Report

Merging #24886 into master will not change coverage.
The diff coverage is n/a.

@@           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% <ø> (ø)	⬆️
#single	41.71% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 42b4c97...6dd2120. Read the comment docs.

[jreback]

I find this change will lead to future consistency issues. The fixture is well named and obvious. It is so clear that something is testing an empty dataframe.

Sure its a trivial thing, but having to actually look inside the body of a test (even if only the first line), makes this not as good as having a fixture.

Again this is going in the wrong direction.

[jbrockmendel]

@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 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.

[jreback]

@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?

[jbrockmendel]

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.

[jreback]

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

[jbrockmendel]

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.

[jreback]

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.

[jbrockmendel]

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?

[jreback]

I am ok with this one. Can you merge master. Also let's use DataFrame() and Series() for the empties.

[jreback]

thanks @jbrockmendel