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_psa.py #1569

Merged
merged 3 commits into from Jul 30, 2017

Conversation

Projects
None yet
2 participants
@utkbansal
Member

utkbansal commented Jul 30, 2017

Fixes #

Changes made in this Pull Request:

PR Checklist

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

utkbansal added some commits Jul 30, 2017

@utkbansal utkbansal changed the title from [Review needed]Pytest Style analysis/test_psa.py to Pytest Style analysis/test_psa.py Jul 30, 2017

err_msg = "Dendrogram dictionary object was not produced"
assert_(type(self.plot_data[1]) is dict, err_msg)
assert type(plot_data[1]) is dict, err_msg

This comment has been minimized.

@jbarnoud

jbarnoud Jul 30, 2017

Contributor

isinstance is probably more appropriate than type(...) is ....

self.path_2)
self.expected = (np.mean(np.amin(self.distance_matrix, axis=0)) +
np.mean(np.amin(self.distance_matrix, axis = 1))) / 2.
# TODO: Fix this hack! without the fixture the test says I passed 3

This comment has been minimized.

@jbarnoud

jbarnoud Jul 30, 2017

Contributor

Could h be a class attribute rather than a fixture?

This comment has been minimized.

@utkbansal

utkbansal Jul 30, 2017

Member

I did try that, but when I make it a class attribute, the test fails saying that h expected 2 parameters but got 3.

This comment has been minimized.

@jbarnoud

jbarnoud Jul 30, 2017

Contributor

It looks indeed like h, when defined as a class attribute, is taken as a method. I guess you could define it as h = staticmethod(PSA.hausdorff_wavg) but I am not sure it is much better than the fixture. Maybe somebody will come with an idea. If not, we'll survive with the fixture.

@jbarnoud jbarnoud added this to the 0.17.0 milestone Jul 30, 2017

@jbarnoud jbarnoud self-assigned this Jul 30, 2017

@jbarnoud jbarnoud merged commit a4e5791 into MDAnalysis:develop Jul 30, 2017

3 checks passed

QuantifiedCode 25 minor issues introduced.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.01%) to 90.406%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment