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.

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_base.py #1547

Merged
merged 2 commits into from Jul 29, 2017
Merged

Conversation

@utkbansal
Copy link
Member

@utkbansal utkbansal commented Jul 23, 2017

Fixes #

Changes made in this Pull Request:

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

def tearDown(self):
del self.u
class TestAnalysisBase(object):

This comment has been minimized.

@kain88-de

kain88-de Jul 25, 2017
Member

no need to have this as a class actually

def test_default(self, u):
an = FrameAnalysis(u.trajectory).run()
assert an.n_frames == len(u.trajectory)
assert_equal(an.frames, list(range(len(u.trajectory))))

This comment has been minimized.

@kain88-de

kain88-de Jul 25, 2017
Member

assert_array_equal

This comment has been minimized.

@kain88-de

kain88-de Jul 25, 2017
Member

please also change for similar cases in this file

This comment has been minimized.

@jbarnoud

jbarnoud Jul 25, 2017
Contributor

See discussion on #1444.

This comment has been minimized.

@utkbansal

utkbansal Jul 26, 2017
Author Member

@kain88-de my understanding is that assert_equal is preferred over assert_array_equal as discussed in 1444?

This comment has been minimized.

@jbarnoud

jbarnoud Jul 26, 2017
Contributor

@utkbansal It is my understanding too.

step=step).run()
ana3 = base.AnalysisFromFunction(simple_function, u.trajectory, u.atoms,
step=step).run()
ana1 = base.AnalysisFromFunction(simple_function, mobile=u.atoms, step=step).run()

This comment has been minimized.

@kain88-de

kain88-de Jul 25, 2017
Member

please don't change this formatting. I'm sure this was done to conform to the 80 char limit of pep8

@kain88-de
Copy link
Member

@kain88-de kain88-de commented Jul 26, 2017

coverage decreased

@utkbansal
Copy link
Member Author

@utkbansal utkbansal commented Jul 27, 2017

@kain88-de Can't figure the drop in coverage, restarting the build.

@utkbansal
Copy link
Member Author

@utkbansal utkbansal commented Jul 27, 2017

@kain88-de @jbarnoud @richardjgowers There is something fishy about the coverage. I have the same 4 lines losing coverage in #1541 too.

@richardjgowers
Copy link
Member

@richardjgowers richardjgowers commented Jul 27, 2017

@utkbansal which 4 lines?

@utkbansal
Copy link
Member Author

@utkbansal utkbansal commented Jul 27, 2017

@richardjgowers 2 lines in core/topologyattrs.py and 2 lines in lib/transformations.py. Have a look at the coverage report at https://coveralls.io/builds/12561272

I have seen the 2 lines in lib/transformations.py loose and gain coverage unpredictably in previous PRs too, but restarting the build usually fixed it.

@utkbansal
Copy link
Member Author

@utkbansal utkbansal commented Jul 29, 2017

@richardjgowers @kain88-de What should I do about the drop?

@utkbansal utkbansal changed the title [WIP]Pytest Style analysis/test_base.py Pytest Style analysis/test_base.py Jul 29, 2017
@kain88-de kain88-de merged commit 0ad6d9b into MDAnalysis:develop Jul 29, 2017
2 of 3 checks passed
2 of 3 checks passed
coverage/coveralls Coverage decreased (-0.03%) to 90.399%
Details
QuantifiedCode No new issues introduced.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants