TST: fixturize series/test_alter_axes.py #22526

h-vetinari · 2018-08-28T06:23:12Z

prep for ENH: Add set_index to Series #22225, in the sense that it preempts (and splits off) the test-related changes that have been required on the DataFrame-side of the PR (see TST/CLN: break up & parametrize tests for df.set_index #22236)
tests modified / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff

In particular, it takes the fixture-like attributes of tests/series/common.TestData and transforms them into fixtures in tests/series/conftest.py, with the eventual goal of replacing all the TestData-attributes in the Series tests, similar to #22471 (I can also open a sister issue to that for the Series tests).

codecov · 2018-08-28T07:11:49Z

Codecov Report

Merging #22526 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #22526      +/-   ##
==========================================
+ Coverage   92.04%   92.04%   +<.01%     
==========================================
  Files         169      169              
  Lines       50782    50782              
==========================================
+ Hits        46742    46744       +2     
+ Misses       4040     4038       -2

Flag	Coverage Δ
#multiple	`90.45% <ø> (ø)`	⬆️
#single	`42.31% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/util/testing.py	`85.96% <0%> (+0.2%)`	⬆️

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 f82c10a...4270363. Read the comment docs.

jreback · 2018-08-29T12:23:16Z

pandas/tests/series/test_alter_axes.py


+class TestSeriesAlterAxes():


inherit from object as more explicit

jreback · 2018-08-29T12:23:52Z

pandas/tests/series/conftest.py

+from pandas import Series
+
+
+@pytest.fixture


can you add doc-strings and meaningful names for these. (e.g. ts -> datetime_series)

Not sure what docstrings you imagine. Something like """A datetime series"""?

see pandas/conftest.py for examples, yes that might be ok, the point is new features need a doc-string

jreback · 2018-08-29T12:23:58Z

pandas/tests/series/conftest.py

+
+
+@pytest.fixture
+def series():


string_series

jreback · 2018-08-29T12:24:03Z

pandas/tests/series/conftest.py

+
+
+@pytest.fixture
+def objSeries():


object_series

jreback · 2018-08-29T12:24:09Z

pandas/tests/series/conftest.py

+
+
+@pytest.fixture
+def empty():


empty_series

h-vetinari · 2018-08-30T06:47:02Z

@jreback

can you add doc-strings and meaningful names for these. (e.g. ts -> datetime_series)

This is the same issue as in #22236 - these "fixture-like" TestData-attributes will need to be replaced in all Series tests, and renaming them now will make it harder to simply replace them (e.g. if someone were to work on a different module, to know that self.series needs to be replaced with string_series).

One possible approach to satisfy both demands would be to open up a sister issue to #22471 for Series, and have a "translation guide" like

self.ts -> datetime_series
self.series -> string_series
self.objSeries -> object_series
self.empty -> empty_series

The same approach could be applied to #22236 / #22471 if you're unhappy with the fixture names coming from TestData there (cc @gfyoung).

h-vetinari · 2018-08-31T06:15:26Z

@jreback

I added the "translations" as requested - it's gonna make transitioning the other modules a fair bit harder, but for only four fixtures, I think it's doable.

jreback

doc comments, ping on green.

jreback · 2018-08-31T09:58:12Z

pandas/tests/series/conftest.py

+from pandas import Series
+
+
+@pytest.fixture


see pandas/conftest.py for examples, yes that might be ok, the point is new features need a doc-string

pep8speaks · 2018-08-31T13:42:48Z

Hello @h-vetinari! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on August 31, 2018 at 13:49 Hours UTC

h-vetinari · 2018-08-31T20:42:08Z

@jreback

All green. This is very similar to #22236 - can I please ask you to take a look there again? I incorporated all your feedback (fixturize, split off warnings into new PR, etc.), and don't know what's left to do - it's been a very drawn-out process to eke out further instructions for what could have been a simple test clean-up.

Based on this review here, I now also added docstrings to the fixtures there; I didn't rename them yet (but added a note in each fixture), because otherwise the fixturization of the other modules would be that much harder (and it's much easier to run a simple search and replace once everything is translated, i.e. #22471 is done).

jreback · 2018-08-31T20:57:53Z

it’s long and drawn out because the PRs have too much unrelated changes in them

1 thing per PR will get u merged much faster with less comments

even things as simple as changing unrelated whatsnew notes

the more code the more i have to go over with a fine toothed comb

further responding to my comments with further questions - while for sure ask if things are unclear
can certainly make things take longer

h-vetinari · 2018-08-31T21:22:59Z

@jreback

For sure, and I appreciate the effort you put in. But I've learned my lesson about the unrelated changes, and I've been splitting them into smaller and smaller pieces.

In #22236, the "unrelated" changes were - respectfully - mostly your requests at cleaning up things not directly related to the PR (except that it was in the same module), and I split off the new warnings as requested as well. The diffs of the last few commits are not very large at all (except the added docstrings now, which I'll happily revert if not necessary) - e.g. starting after your last review: https://github.com/pandas-dev/pandas/pull/22236/commits/9121d4912cdcd7f8a7e3336fc385bf00ccab93ad

h-vetinari · 2018-09-04T06:12:23Z

@jreback anything left to do on this? Someone wants to already start working on fixturising the rest of the series tests, see #22550

jreback

small comments ping on green.

@h-vetinari the pings are fine, but if someone wants to work on things based on this nothing is stopping them, they can rebase off of this PR, this is a quite common workflow. This will be merged after all comments are satisfied. Pls be patient. We have a tremendous amount of PRs

jreback · 2018-09-04T11:48:06Z

pandas/tests/series/conftest.py

+    """
+    Fixture for Series of floats with DatetimeIndex
+
+    See pandas.util.testing.makeTimeSeries


I don't think these See .. comments make sense here, just keep the summary line.

Removed those comments

h-vetinari · 2018-09-04T23:08:26Z

@jreback

@h-vetinari the pings are fine, but if someone wants to work on things based on this nothing is stopping them, they can rebase off of this PR, this is a quite common workflow. This will be merged after all comments are satisfied. Pls be patient. We have a tremendous amount of PRs

Yeah, I get there's lots of traffic. I mentioned this because this PR was basically finished and it's not a "beginner workflow" to rebase off an open PR.

h-vetinari · 2018-09-05T23:29:53Z

@jreback

small comments ping on green.

ping on green

jreback · 2018-09-05T23:43:43Z

thanks!

jreback added the Testing pandas testing functions or related to the test suite label Aug 29, 2018

jreback requested changes Aug 29, 2018

View reviewed changes

h-vetinari mentioned this pull request Aug 31, 2018

TST/CLN: remove TestData from series-tests; replace with fixtures #22550

Closed

20 tasks

h-vetinari force-pushed the tst_series_alter_axes branch 2 times, most recently from 39b251d to 38afdfd Compare August 31, 2018 09:29

jreback requested changes Aug 31, 2018

View reviewed changes

jreback added this to the 0.24.0 milestone Aug 31, 2018

h-vetinari force-pushed the tst_series_alter_axes branch 2 times, most recently from 49a91c6 to c917d81 Compare August 31, 2018 13:49

h-vetinari mentioned this pull request Sep 1, 2018

TST/CLN: break up & parametrize tests for df.set_index #22236

Merged

jreback requested changes Sep 4, 2018

View reviewed changes

h-vetinari added 3 commits September 5, 2018 13:48

TST: fixturize series/test_alter_axes.py

b099f6c

Review (jreback)

70ee3bf

Review (jreback)

4270363

h-vetinari force-pushed the tst_series_alter_axes branch from a355f5f to 4270363 Compare September 5, 2018 11:49

jreback approved these changes Sep 5, 2018

View reviewed changes

jreback merged commit 46abe18 into pandas-dev:master Sep 5, 2018

h-vetinari deleted the tst_series_alter_axes branch September 9, 2018 21:22

h-vetinari mentioned this pull request Sep 18, 2018

ENH: Add set_index to Series #22225

Closed

5 tasks

aeltanawy pushed a commit to aeltanawy/pandas that referenced this pull request Sep 20, 2018

TST: fixturize series/test_alter_axes.py (pandas-dev#22526)

25030e2

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

TST: fixturize series/test_alter_axes.py (pandas-dev#22526)

27d3bc8

h-vetinari mentioned this pull request Jan 23, 2019

TST: Remove subset of singleton fixtures #24873

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TST: fixturize series/test_alter_axes.py #22526

TST: fixturize series/test_alter_axes.py #22526

h-vetinari commented Aug 28, 2018 •

edited

Loading

codecov bot commented Aug 28, 2018 •

edited

Loading

jreback Aug 29, 2018

jreback Aug 29, 2018

h-vetinari Aug 31, 2018

jreback Aug 31, 2018

jreback Aug 29, 2018

jreback Aug 29, 2018

jreback Aug 29, 2018

h-vetinari commented Aug 30, 2018 •

edited

Loading

h-vetinari commented Aug 31, 2018

jreback left a comment

jreback Aug 31, 2018

pep8speaks commented Aug 31, 2018 •

edited

Loading

h-vetinari commented Aug 31, 2018

jreback commented Aug 31, 2018 •

edited

Loading

h-vetinari commented Aug 31, 2018 •

edited

Loading

h-vetinari commented Sep 4, 2018 •

edited

Loading

jreback left a comment

jreback Sep 4, 2018

h-vetinari Sep 4, 2018

h-vetinari commented Sep 4, 2018

h-vetinari commented Sep 5, 2018 •

edited

Loading

jreback commented Sep 5, 2018


		class TestSeriesAlterAxes():



		@pytest.fixture
		def series():



		@pytest.fixture
		def objSeries():



		@pytest.fixture
		def empty():

TST: fixturize series/test_alter_axes.py #22526

TST: fixturize series/test_alter_axes.py #22526

Conversation

h-vetinari commented Aug 28, 2018 • edited Loading

codecov bot commented Aug 28, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-vetinari commented Aug 30, 2018 • edited Loading

h-vetinari commented Aug 31, 2018

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pep8speaks commented Aug 31, 2018 • edited Loading

Comment last updated on August 31, 2018 at 13:49 Hours UTC

h-vetinari commented Aug 31, 2018

jreback commented Aug 31, 2018 • edited Loading

h-vetinari commented Aug 31, 2018 • edited Loading

h-vetinari commented Sep 4, 2018 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-vetinari commented Sep 4, 2018

h-vetinari commented Sep 5, 2018 • edited Loading

jreback commented Sep 5, 2018

h-vetinari commented Aug 28, 2018 •

edited

Loading

codecov bot commented Aug 28, 2018 •

edited

Loading

h-vetinari commented Aug 30, 2018 •

edited

Loading

pep8speaks commented Aug 31, 2018 •

edited

Loading

jreback commented Aug 31, 2018 •

edited

Loading

h-vetinari commented Aug 31, 2018 •

edited

Loading

h-vetinari commented Sep 4, 2018 •

edited

Loading

h-vetinari commented Sep 5, 2018 •

edited

Loading