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_hole.py #1539

Merged
merged 3 commits into from Jul 26, 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?

from MDAnalysisTests.datafiles import PDB_HOLE, MULTIPDB_HOLE
from MDAnalysisTests import (executable_not_found, module_not_found,
tempdir, in_dir)

This comment has been minimized.

@kain88-de

kain88-de Jul 25, 2017
Member

import order is

  1. python std library
  2. external packages like numpy/pandas/...
  3. mdanalysis specific packages
  4. testing specific packages

from numpy.testing import (TestCase, dec,
assert_equal, assert_almost_equal,
from MDAnalysisTests import (executable_not_found, tempdir, in_dir)

This comment has been minimized.

@kain88-de

kain88-de Jul 25, 2017
Member

please remove the tempdir dependency

for ts in cls.universe.trajectory[cls.start:cls.stop]]
@pytest.fixture()
def H(self, universe):
with tempdir.in_tempdir():

This comment has been minimized.

@kain88-de

kain88-de Jul 25, 2017
Member

use a tmpdir_factory here

class TestHOLE(TestCase):
filename = PDB_HOLE

@dec.skipif(executable_not_found("hole"), msg="Test skipped because HOLE not found")
def setUp(self):

This comment has been minimized.

@kain88-de

kain88-de Jul 25, 2017
Member

should be a fixture

@utkbansal utkbansal changed the title [WIP]Pytest Style analysis/test_hole.py Pytest Style analysis/test_hole.py Jul 25, 2017
@utkbansal
Copy link
Member Author

@utkbansal utkbansal commented Jul 25, 2017

@kain88-de Anything else?

@dec.skipif(executable_not_found("hole"), msg="Test skipped because HOLE not found")
def setUp(self):

@pytest.fixture()

This comment has been minimized.

@kain88-de

kain88-de Jul 25, 2017
Member

Shouldn't this be a static method for pytest to work?

This comment has been minimized.

@utkbansal

utkbansal Jul 26, 2017
Author Member

I don't think so. I can move filename = PDB_HOLE to the fixture though and make it a staticmethod.

This comment has been minimized.

@kain88-de

kain88-de Jul 26, 2017
Member

so far you made all fixtures in a class static. I was wondering why this isn't necessary here.

This comment has been minimized.

@utkbansal

utkbansal Jul 26, 2017
Author Member

@kain88-de I sometimes need to access self from within the fixture. That is when I don't make it static.

@kain88-de kain88-de merged commit 5672a7a into MDAnalysis:develop Jul 26, 2017
3 checks passed
3 checks passed
QuantifiedCode 7 minor issues introduced.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.02%) to 90.406%
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

2 participants