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

REF/TST: Add pytest idiom to reshape/test_tile #24107

Merged
merged 2 commits into from
Dec 6, 2018
Merged

REF/TST: Add pytest idiom to reshape/test_tile #24107

merged 2 commits into from
Dec 6, 2018

Conversation

gfyoung
Copy link
Member

@gfyoung gfyoung commented Dec 5, 2018

No description provided.

@gfyoung gfyoung added Refactor Internal refactoring of code Testing pandas testing functions or related to the test suite labels Dec 5, 2018
@gfyoung gfyoung added this to the 0.24.0 milestone Dec 5, 2018
@pep8speaks
Copy link

pep8speaks commented Dec 5, 2018 •
edited
Loading

Hello @gfyoung! Thanks for updating the PR.

Comment last updated on December 06, 2018 at 05:21 Hours UTC

@codecov
Copy link

codecov bot commented Dec 5, 2018 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #24107   +/-   ##
=======================================
  Coverage    92.2%    92.2%           
=======================================
  Files         162      162           
  Lines       51701    51701           
=======================================
  Hits        47672    47672           
  Misses       4029     4029
Flag Coverage Δ
#multiple 90.6% <ø> (ø) ⬆️
#single 43.03% <ø> (ø) ⬆️

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 b841374...25c680c. Read the comment docs.

@pandas-dev pandas-dev deleted a comment from codecov bot Dec 5, 2018
@jreback
Copy link
Contributor

jreback commented Dec 5, 2018

can you organize into 3 sections: basic, cut, qcut and either: put each in a class, or just put a section break between (maybe better)?

@gfyoung
Copy link
Member Author

gfyoung commented Dec 5, 2018

put each in a class

Kind of defeats the purpose of converting to pytest idiom 😉

I actually think we can just break up into two modules (test_cut and test_qcut).

@WillAyd
Copy link
Member

WillAyd commented Dec 5, 2018

Kind of defeats the purpose of converting to pytest idiom 😉

What's the reasoning of that? Plenty of examples on their documentation for using classes to organize; curious where its established as non-idiomatic to use classes

@jreback
Copy link
Contributor

jreback commented Dec 5, 2018

@gfyoung we do use classes, though I think i prefer separate files generally. for this case could be either way.

@gfyoung
Copy link
Member Author

gfyoung commented Dec 5, 2018 •
edited
Loading

What's the reasoning of that? Plenty of examples on their documentation for using classes to organize

@WillAyd : I'm not saying that classes are not idiomatic. They are just less pytest-idiomatic than functions. Functional testing is the core feature of pytest (vs. something like unittest).

In this case, there was not real reason to have OOP overhead for these tests, and I agree that with @jreback that separate test files would do the trick here.

@gfyoung gfyoung force-pushed the test-tile-pytest-idiom branch from ccb50ec to 8bb0b9f Compare December 6, 2018 00:46
@gfyoung gfyoung force-pushed the test-tile-pytest-idiom branch from 8bb0b9f to 25c680c Compare December 6, 2018 05:21
@gfyoung
Copy link
Member Author

gfyoung commented Dec 6, 2018

@jreback @WillAyd : I split up the test into two files, which should be a little easier to read. PTAL.

@jreback jreback merged commit b78aa8d into pandas-dev:master Dec 6, 2018
@jreback
Copy link
Contributor

jreback commented Dec 6, 2018

thanks @gfyoung

TomAugspurger added a commit to TomAugspurger/pandas that referenced this pull request Dec 6, 2018
@TomAugspurger
Squashed commit of the following:
1f463a1 
commit 28c61d770f6dfca6857fd0fa6979d4119a31129e
Author: Tom Augspurger <tom.w.augspurger@gmail.com>
Date:   Thu Dec 6 12:18:19 2018 -0600

    uncomment

commit bae2e322523efc73a1344464f51611e2dc555ccb
Author: Tom Augspurger <tom.w.augspurger@gmail.com>
Date:   Thu Dec 6 12:17:09 2018 -0600

    maybe fixes

commit 6cb4db05c9d6ceba3794096f0172cae5ed5f6019
Author: Tom Augspurger <tom.w.augspurger@gmail.com>
Date:   Thu Dec 6 09:57:37 2018 -0600

    we back

commit d97ab57fb32cb23371169d9ed659ccfac34cfe45
Merge: a117de4 b78aa8d
Author: Tom Augspurger <tom.w.augspurger@gmail.com>
Date:   Thu Dec 6 09:51:51 2018 -0600

    Merge remote-tracking branch 'upstream/master' into disown-tz-only-rebased2

commit b78aa8d
Author: gfyoung <gfyoung17+GitHub@gmail.com>
Date:   Thu Dec 6 07:18:44 2018 -0500

    REF/TST: Add pytest idiom to reshape/test_tile (pandas-dev#24107)

commit 2993b8e
Author: gfyoung <gfyoung17+GitHub@gmail.com>
Date:   Thu Dec 6 07:17:55 2018 -0500

    REF/TST: Add more pytest idiom to scalar/test_nat (pandas-dev#24120)

commit b841374
Author: evangelineliu <hsiyinliu@gmail.com>
Date:   Wed Dec 5 18:21:46 2018 -0500

    BUG: Fix concat series loss of timezone (pandas-dev#24027)

commit 4ae63aa
Author: jbrockmendel <jbrockmendel@gmail.com>
Date:   Wed Dec 5 14:44:50 2018 -0800

    Implement DatetimeArray._from_sequence (pandas-dev#24074)

commit 2643721
Author: jbrockmendel <jbrockmendel@gmail.com>
Date:   Wed Dec 5 14:43:45 2018 -0800

    CLN: Follow-up to pandas-dev#24100 (pandas-dev#24116)

commit 8ea7744
Author: chris-b1 <cbartak@gmail.com>
Date:   Wed Dec 5 14:21:23 2018 -0600

    PERF: ascii c string functions (pandas-dev#23981)

commit cb862e4
Author: jbrockmendel <jbrockmendel@gmail.com>
Date:   Wed Dec 5 12:19:46 2018 -0800

    BUG: fix mutation of DTI backing Series/DataFrame (pandas-dev#24096)

commit aead29b
Author: topper-123 <contribute@tensortable.com>
Date:   Wed Dec 5 19:06:00 2018 +0000

    API: rename MultiIndex.labels to MultiIndex.codes (pandas-dev#23752)
@gfyoung gfyoung deleted the test-tile-pytest-idiom branch December 7, 2018 02:42
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@gfyoung @Pingviinituutti
REF/TST: Add pytest idiom to reshape/test_tile (pandas-dev#24107)
d3ca05d
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@gfyoung @Pingviinituutti
REF/TST: Add pytest idiom to reshape/test_tile (pandas-dev#24107)
bc09b98
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Refactor Internal refactoring of code Testing pandas testing functions or related to the test suite
Projects
None yet
Milestone
0.24.0
Development

Successfully merging this pull request may close these issues.
4 participants
@gfyoung @pep8speaks @jreback @WillAyd