Skip to content

test for writing DX files from density (Issue #544)#563

Merged
kain88-de merged 2 commits into
developfrom
issue/544
Dec 14, 2015
Merged

test for writing DX files from density (Issue #544)#563
kain88-de merged 2 commits into
developfrom
issue/544

Conversation

@orbeckst

@orbeckst orbeckst commented Dec 5, 2015

Copy link
Copy Markdown
Member

Requires a 0.3.1 release of GridDataFormats, see MDAnalysis/GridDataFormats#24.

@orbeckst

orbeckst commented Dec 5, 2015

Copy link
Copy Markdown
Member Author

rebased against develop (in particular with #552 fixed).

@orbeckst

orbeckst commented Dec 8, 2015

Copy link
Copy Markdown
Member Author

Rebased against develop and force-pushed. Added new requirement for GridDataFormats >= 0.3.1.

@orbeckst

orbeckst commented Dec 8, 2015

Copy link
Copy Markdown
Member Author

No idea why, on travis, python appears to be unable to find GridDataFormats, even though it is clearly being installed (in version 0.3.1):

from gridData import Grid

fails in MDAnalysis.analysis.density.

@kain88-de

Copy link
Copy Markdown
Member

@orbeckst did you check locally of the gridData package can be loaded in a new virtualenv?

@orbeckst

orbeckst commented Dec 9, 2015

Copy link
Copy Markdown
Member Author

rebased against develop

@orbeckst

Copy link
Copy Markdown
Member Author

Installing GridDataFormats into fresh virtualenv worked and all tests passed.

@orbeckst

Copy link
Copy Markdown
Member Author

rebased against develop

@orbeckst

Copy link
Copy Markdown
Member Author

Now part of the tests pass in build #1038:

  • SETUP=full CYTHONIZE=true: passes (build #1038.1)
  • SETUP=minimal CYTHONIZE=false: fails because gridData could not be imported (build #1038.2)

I don't quite understand the failure, though, because the travis config always does a pip install GridDataFormats, regardless of the SETUP variable.

@kain88-de

Copy link
Copy Markdown
Member

Yeah I also don't quite get the error. You could try to start python and import gridData independent of MDAnalysis. Adding a line like python -c import gridData should do that.

@orbeckst

Copy link
Copy Markdown
Member Author

@kain88-de thanks for the idea: now I know why the minimal fails: GridDataFormat wants scipy.

$ python -c 'import gridData; print(gridData.__version__)'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/travis/miniconda/envs/pyenv/lib/python2.7/site-packages/gridData/__init__.py", line 191, in <module>
    from .core import Grid
  File "/home/travis/miniconda/envs/pyenv/lib/python2.7/site-packages/gridData/core.py", line 30, in <module>
    from scipy import ndimage
ImportError: No module named scipy

So I will decorate the tests.

@orbeckst

Copy link
Copy Markdown
Member Author

... or see if I can make scipy optional in GridDataFormats, i.e. only loaded if one wants to do the interpolations.

@kain88-de

Copy link
Copy Markdown
Member

Currently this is not possible. The ndimage function is already called at the initialization of the Grid object.

@orbeckst

Copy link
Copy Markdown
Member Author

ah, crap... this fails at the top level as soon as analysis.density is imported.

Well, wait for MDAnalysis/GridDataFormats#25 ...

@orbeckst

Copy link
Copy Markdown
Member Author

Just released GridDataFormat 0.3.2: will update and rebase this branch and then it should work...

- The new release 0.3.1 of GridDataFormats contains the fix for issue MDAnalysis/GridDataFormats#21
  which is needed to fix issue #544.
- The release 0.3.2 allows importing of gridData without scipy.ndimage present
  (MDAnalysis/GridDataFormats#25): this allows the minimal tests to pass
@orbeckst orbeckst changed the title WIP: test for writing DX files from density (Issue #544) test for writing DX files from density (Issue #544) Dec 12, 2015
@orbeckst orbeckst assigned kain88-de and unassigned orbeckst Dec 12, 2015
@orbeckst

Copy link
Copy Markdown
Member Author

(I am a bit miffed that QC makes the PR look as if it does not pass the tests... most of the QC issues are things that I will simply ignore.)

@mnmelo

mnmelo commented Dec 13, 2015

Copy link
Copy Markdown
Member

How I see it we have to tweak as we go. Any given test can be disabled, so go ahead and disable the ones you think won't apply to our style (or tell me which and I'll do it).

As to the build "failing", I think it's harmless, and draws the attention of the PR reviewer to possible code issues. When we prune to leave only the tests we consider important a failure will be much more meaningful.

@orbeckst

Copy link
Copy Markdown
Member Author

I reviewed the QC issue and I will not fix any of the reported issues --- they all relate to how we write our tests and I don't think that there's anything wrong with it,

@kain88-de, please consider the PR complete from my end (short of other problems you or anyone else might identify).

kain88-de added a commit that referenced this pull request Dec 14, 2015
test for writing DX files from density (Issue #544)
@kain88-de kain88-de merged commit b01f974 into develop Dec 14, 2015
@orbeckst orbeckst deleted the issue/544 branch December 14, 2015 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants