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.

Sign up for GitHub

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

Added exception and unittest for dist_mat_to_vec of analysis/psa.py (Part of #597) #1183

Merged
merged 15 commits into from
Feb 4, 2017
Merged

Added exception and unittest for dist_mat_to_vec of analysis/psa.py (Part of #597) #1183

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9a49244
Added exception and unittest for function dist_mat_to_vec of analysis…
vedantrathore Jan 22, 2017
ceb1618
Merge branch 'develop' of https://github.com/MDAnalysis/mdanalysis in…
vedantrathore Jan 22, 2017
1b04205
Merge branch 'develop' of https://github.com/MDAnalysis/mdanalysis in…
vedantrathore Jan 23, 2017
3af71be
Added exception and unittest for dist_mat_to_vec of analysis/psa.py (…
vedantrathore Jan 23, 2017
bf8765b
Added exception and unittest for dist_mat_to_vec of analysis/psa.py (…
vedantrathore Jan 23, 2017
cda6c57
Added exception and unittest for dist_mat_to_vec of analysis/psa.py (…
vedantrathore Jan 23, 2017
dcc6168
Adds validation for dist_mat_to_vec function inputs
vedantrathore Jan 28, 2017
cecc350
Merge branch 'develop' of https://github.com/MDAnalysis/mdanalysis in…
vedantrathore Jan 28, 2017
3943ddc
Added exception and unittest for dist_mat_to_vec of analysis/psa.py (…
vedantrathore Jan 31, 2017
b706493
Merge branch 'develop' of https://github.com/MDAnalysis/mdanalysis in…
vedantrathore Jan 31, 2017
f12458e
Added exception and unittest for dist_mat_to_vec of analysis/psa.py (…
vedantrathore Jan 31, 2017
ece534a
Added exception and unittest for dist_mat_to_vec of analysis/psa.py (…
vedantrathore Feb 2, 2017
36acf58
Added exception and unittest for dist_mat_to_vec of analysis/psa.py (…
vedantrathore Feb 3, 2017
376b0b1
Added exception and unittest for dist_mat_to_vec of analysis/psa.py (…
vedantrathore Feb 3, 2017
bd94007
Added exception and unittest for dist_mat_to_vec of analysis/psa.py (…
vedantrathore Feb 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package/MDAnalysis/analysis/psa.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@
from six.moves import range, cPickle

import numpy as np
import warnings
import warnings,numbers

import MDAnalysis
import MDAnalysis.analysis.align
Expand Down Expand Up @@ -575,7 +575,7 @@ def dist_mat_to_vec(N, i, j):
int, index (of the matrix element) in the corresponding distance vector
"""

if not (isinstance(N, int) or isinstance(i, int) or isinstance(j, int)):
if not (isinstance(N, numbers.Integral) or isinstance(i, numbers.Integral) or isinstance(j, numbers.Integral)):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should add tests as well for the numpy integers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh forgot that, my bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kain88-de I was thinking about how to check whether input is numpy integer or not, I think we are only checking whether the input is not an integer, How should I validate whether the input is valid integer?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current check is testing if the input is an integer. In you test you should just make sure that it's ok to use numpy integers like np.int32 in the function. That is important if one wants to use integers from numpy arrays.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What @kain88-de means is that you should add a few lines in test_psa.py with numpy integer as inputs. The function should work of we give N=numpy.int32(4) or N=numpy.int16(4). and this has to be tested. It is likely that the input values for dist_mat_to_vec will come from a numpy array at some point, then their type will not be a plain int.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kain88-de @jbarnoud I think this is what you meant, right?

raise ValueError("N, i, j all must be of type int")

if i < 0 or j < 0 or N < 2:
Expand Down