Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixturize tests/frame/test_constructors.py #25635

Merged
merged 6 commits into from
Jun 28, 2019
Merged

Fixturize tests/frame/test_constructors.py #25635

merged 6 commits into from
Jun 28, 2019

Conversation

h-vetinari
Copy link
Contributor

One more steps towards #22471. Again, needing to re-add some fixtures that were removed in #24885.

This was referenced Mar 10, 2019
Merged
Closed
jreback
jreback reviewed Mar 10, 2019
View reviewed changes
pandas/tests/frame/conftest.py Outdated
@@ -127,6 +127,22 @@ def timezone_frame():
return df


@pytest.fixture
def datetime_series():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't add any more here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not here for now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jreback
See above:

PS. As noted in the other threads, this is a TestData-"fixture" that's used in several modules.

This is used also used in #25627, for example

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think these fixtures useful; if you really really want them put them in the actual test file for now

jreback
jreback requested changes Mar 10, 2019
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe my comments are crossing with your reading of them.

@jreback jreback added the Testing pandas testing functions or related to the test suite label Mar 10, 2019
@codecov
Copy link

codecov bot commented Mar 10, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #25635   +/-   ##
=======================================
  Coverage   91.26%   91.26%           
=======================================
  Files         173      173           
  Lines       52968    52968           
=======================================
  Hits        48339    48339           
  Misses       4629     4629
Flag Coverage Δ
#multiple 89.83% <ø> (ø) ⬆️
#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 df771cc...f10b72c. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 10, 2019 •
edited
Loading

Codecov Report

Merging #25635 into master will increase coverage by 49.3%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #25635      +/-   ##
==========================================
+ Coverage   41.95%   91.26%   +49.3%     
==========================================
  Files         180      173       -7     
  Lines       50765    52982    +2217     
==========================================
+ Hits        21300    48355   +27055     
+ Misses      29465     4627   -24838
Flag Coverage Δ
#multiple 89.83% <ø> (?)
#single 41.76% <ø> (-0.2%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/compat/__init__.py 58.03% <0%> (-33.97%) ⬇️
pandas/plotting/_misc.py 38.68% <0%> (-4.56%) ⬇️
pandas/io/clipboard/clipboards.py 30.58% <0%> (-4.2%) ⬇️
pandas/core/api.py 100% <0%> (ø) ⬆️
pandas/core/groupby/__init__.py 100% <0%> (ø) ⬆️
pandas/_libs/__init__.py 100% <0%> (ø) ⬆️
pandas/io/api.py 100% <0%> (ø) ⬆️
pandas/_libs/tslibs/__init__.py 100% <0%> (ø) ⬆️
pandas/io/clipboards.py 100% <0%> (ø) ⬆️
... and 163 more

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 e9f9ca1...95b2b34. Read the comment docs.

@h-vetinari
Copy link
Contributor Author

@jreback
All green.

PS. As noted in the other threads, this is a TestData-"fixture" that's used in several modules.

jreback
jreback requested changes Mar 10, 2019
View reviewed changes
pandas/tests/frame/conftest.py Outdated
@@ -127,6 +127,22 @@ def timezone_frame():
return df


@pytest.fixture
def datetime_series():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not here for now

h-vetinari
h-vetinari commented Mar 11, 2019
View reviewed changes
Copy link
Contributor Author

@h-vetinari h-vetinari left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll reiterate from another thread, that with these fixturization PRs, I will:

pandas/tests/frame/conftest.py Outdated
@@ -127,6 +127,22 @@ def timezone_frame():
return df


@pytest.fixture
def datetime_series():
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jreback
See above:

PS. As noted in the other threads, this is a TestData-"fixture" that's used in several modules.

This is used also used in #25627, for example

@h-vetinari
Copy link
Contributor Author

Just for context, these fixtures had already been there (since #22236), and were only recently removed in #24885, even though I noted that there was an ongoing (is somewhat stalled) translation effort.

Can we leave the re-org for later and not re-litigate the fixtures that we already agreed to use in #22236?

jreback
jreback requested changes Mar 20, 2019
View reviewed changes
pandas/tests/frame/conftest.py Outdated
@@ -127,6 +127,22 @@ def timezone_frame():
return df


@pytest.fixture
def datetime_series():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think these fixtures useful; if you really really want them put them in the actual test file for now

@jreback
Copy link
Contributor

jreback commented Mar 20, 2019

pls merge master

@h-vetinari
Copy link
Contributor Author

re datetime_series:

@jreback: i don't think these fixtures useful; if you really really want them put them in the actual test file for now

I agree with you, but currently tests.frame.common.TestData.ts1 is still used in several places, and will have to be on the level of tests.frame.conftest - another PR that needs the same fixture is #25627

@jreback
Copy link
Contributor

jreback commented Apr 20, 2019

closing as stale

@jreback jreback closed this Apr 20, 2019
@h-vetinari
Copy link
Contributor Author

@jreback

I had merged master like you asked, this PR is not stale. Please reopen.

@h-vetinari
Copy link
Contributor Author

@jreback @gfyoung @simonjayhawkins
Can you please reopen this PR? Now that #25627 is in, this will be easier.

@h-vetinari
Copy link
Contributor Author

h-vetinari commented Jun 28, 2019 •
edited
Loading

Ok, merged and cleaned. Got rid of the datetime_series and datetime_series_short fixture in the same way as @jreback in this commit of #25627.

Also: Thanks for reopening @simonjayhawkins.

@jreback jreback added this to the 0.25.0 milestone Jun 28, 2019
@jreback
Copy link
Contributor

jreback commented Jun 28, 2019

lgtm.

@jreback jreback merged commit 4ef793f into pandas-dev:master Jun 28, 2019
@jreback
Copy link
Contributor

jreback commented Jun 28, 2019

thanks @h-vetinari

@h-vetinari h-vetinari deleted the fixturize_frame_constructors branch June 28, 2019 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Testing pandas testing functions or related to the test suite
Projects
None yet
Milestone
0.25.0
Development

Successfully merging this pull request may close these issues.
3 participants
@h-vetinari @jreback @simonjayhawkins