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: auxiliary/base.py #1492

Merged
merged 4 commits into from Jul 18, 2017

Conversation

Projects
None yet
4 participants
@utkbansal
Member

utkbansal commented Jul 15, 2017

Fixes #

Changes made in this Pull Request:

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?
import MDAnalysis as mda
from MDAnalysisTests.datafiles import (COORDINATES_XTC, COORDINATES_TOPOLOGY)
@raises(ValueError)
@pytest.mark.raises(exception=ValueError)

This comment has been minimized.

@kain88-de

kain88-de Jul 16, 2017

Member

please use the pytest.raises context manager

@utkbansal

This comment has been minimized.

Member

utkbansal commented Jul 16, 2017

That's odd. The tests pass on my system.

@utkbansal

This comment has been minimized.

Member

utkbansal commented Jul 16, 2017

@kain88-de @richardjgowers @jbarnoud base.py isn't collected for tests and hence test_get_bad_auxreader_format_raises_ValueError isn't executed. Should I move it to test_xvg.py or should we rename base.py?

utkbansal added some commits Jul 16, 2017

@utkbansal

This comment has been minimized.

Member

utkbansal commented Jul 16, 2017

@kain88-de Is it normal that I have to uninstall and reinstall MDAnalysis & MDAnalysisTests to reflect the changes I make in the tests?

@utkbansal

This comment has been minimized.

Member

utkbansal commented Jul 16, 2017

@kain88-de @richardjgowers @jbarnoud On running test_iterate_as_auxiliary_from_trajectory I get the error ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all() The error seems logical, how is it supposed to even compare it, there is ambiguity. How should I fix this?

@mnmelo

This comment has been minimized.

Member

mnmelo commented Jul 16, 2017

That's odd, but I guess it depends on how exactly you're installing/running. Checking MDAnalysisTests.__path__ might also shed light on why you're not getting the behavior you think you should be getting.

@utkbansal

This comment has been minimized.

Member

utkbansal commented Jul 16, 2017

@mnmelo This is what I see.

>>> MDAnalysisTests.__path__
['MDAnalysisTests']

I have setup using the following commands

pip install -e package/
pip install -e testsuite/
@mnmelo

This comment has been minimized.

Member

mnmelo commented Jul 16, 2017

Assuming you're in your develop tree, then that seems ok, and you shouldn't need reinstalls. What exactly is the symptom that prompts you to reinstall MDAnalysis?

@jbarnoud

This comment has been minimized.

Contributor

jbarnoud commented Jul 16, 2017

@mnmelo

This comment has been minimized.

Member

mnmelo commented Jul 16, 2017

Ah yes, a reinstall will be needed (once) if you pull any changes to MDAnalysis that involve changes to the C/Cython parts of the code. Was that the case, @utkbansal?

@mnmelo

This comment has been minimized.

Member

mnmelo commented Jul 16, 2017

As to the test failure, there's a == comparison between arrays, which returns a bool array that can't be converted to a single bool value. I can't readily see the code and contents of the aux data being compared, but it seems either the wrong data are being compared or not being frame-indexed properly.

@utkbansal

This comment has been minimized.

Member

utkbansal commented Jul 16, 2017

What exactly is the symptom that prompts you to reinstall MDAnalysis?

The basic symptom is that I make changes to tests and there is no effect when I run them. And that sometimes(like in this PR) causes the Travis build to fail though tests on my local system pass.

Ah yes, a reinstall will be needed (once) if you pull any changes to MDAnalysis that involve changes to the C/Cython parts of the code. Was that the case, @utkbansal?

This might have been the problem, but in this PR I haven't touched any .pyx files.

@kain88-de

This comment has been minimized.

Member

kain88-de commented Jul 16, 2017

@utkbansal I recommend you work in a virtual env or conda-env. When you use your global env and one time installed without the -e flag you might see such problems.

@utkbansal

This comment has been minimized.

Member

utkbansal commented Jul 16, 2017

@kain88-de I'm already using a virtualenv.

@mnmelo

This comment has been minimized.

Member

mnmelo commented Jul 18, 2017

As for the failing array comparisons: you blindly replaced all assert_equal by assert. The two are not equivalent: numpy's assert_equal takes care of comparing arrays element-wise (it was also decided in #1444 to keep the more complete asserts from numpy, except assert_). You should roll back those changes, except maybe for cases where they were being used to compare single values.

@kain88-de kain88-de merged commit 192e75b into MDAnalysis:develop Jul 18, 2017

3 checks passed

QuantifiedCode No new issues introduced.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.04%) to 90.4%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment