-
Notifications
You must be signed in to change notification settings - Fork 709
Pytest Style analysis/test_nuclinfo.py #1549
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.
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
Pytest Style analysis/test_nuclinfo.py #1549
Conversation
|
||
@staticmethod | ||
@pytest.fixture() | ||
def u(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we increase the scope of this fixture?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@richardjgowers Increase to what level?
@richardjgowers Done. Dropped the class too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each method is covered by two tests. Each of these pairs can be grouped in a parametrized test.
def test_minor_pair_2(self): | ||
val = nuclinfo.minor_pair(self.u, 20, 5, seg1='RNAA', seg2='RNAA') | ||
assert_almost_equal(val, 4.356, decimal=3) | ||
def test_minor_pair_1(u): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_minor_pair_1
and test_minor_pair_2
can be grouped in a parametrized test_minor_pair
.
def test_phase_cp_2(self): | ||
val = nuclinfo.phase_cp(self.u, seg='RNAA', i=21) | ||
assert_almost_equal(val, 16.199, decimal=3) | ||
def test_major_pair_1(u): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_major_pair_*
can be grouped in a parametrized test.
Fixes #
Changes made in this Pull Request:
PR Checklist