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

package/AUTHORS

@@ -81,6 +81,7 @@ Chronological list of authors
   - Shantanu Srivastava
  2017
   - Utkarsh Bansal
+  - Vedant Rathore
 
 External code
 -------------

package/CHANGELOG

@@ -15,7 +15,7 @@ The rules for this file:
 ------------------------------------------------------------------------------
 ??/??/16 kain88-de, fiona-naughton, richardjgowers, tyler.je.reddy, jdetle
          euhruska, orbeckst, rbrtdlg, jbarnoud, wouterboomsma, shanmbic,
-         dotsdl, manuel.nuno.melo, utkbansal
+         dotsdl, manuel.nuno.melo, utkbansal, vedantrathore
 
   * 0.16.0
 
@@ -61,6 +61,7 @@ Enhancements
   * Added groupby method to Group objects. (PR #1112)
 
 Fixes
+  * Argument validation of dist_mat_to_vec is fixed (#597 PR #1183)
   * Give correct error when the topology file format is not recognized (Issue #982)
   * Give correct error when file doesn't exist/ has bad permissions (Issue #981)
   * Improvement of analysis/waterdynamics module (Issue #935)

package/MDAnalysis/analysis/psa.py

@@ -212,6 +212,7 @@
 from six.moves import range, cPickle
 
 import numpy as np
+import warnings,numbers
 
 import MDAnalysis
 import MDAnalysis.analysis.align
@@ -573,7 +574,15 @@ def dist_mat_to_vec(N, i, j):
     :Returns:
       int, index (of the matrix element) in the corresponding distance vector
     """
-    if i > N or j > N:
+
+    if not (isinstance(N, numbers.Integral) or isinstance(i, numbers.Integral) or isinstance(j, numbers.Integral)):
+        raise ValueError("N, i, j all must be of type int")
+
+    if i < 0 or j < 0 or N < 2:
+        error_str = "Matrix indices are invalid; i and j must be greater than 0 and N must be greater the 2"
+        raise ValueError(error_str)
+
+    if (j > i and (i > N - 1 or j > N)) or (j < i and (i > N or j > N - 1)):
         err_str = "Matrix indices are out of range; i and j must be less than"  \
                 + " N = {0:d}".format(N)
         raise ValueError(err_str)
@@ -583,7 +592,7 @@ def dist_mat_to_vec(N, i, j):
         warn_str = "Column index entered (j = {:d} is smaller than row index"   \
                  + " (i = {:d}). Using symmetric element in upper triangle of"  \
                  + " distance matrix instead: i --> j, j --> i"
-        print(warn_str)
+        warnings.warn(warn_str.format(j, i))
         return (N*j) + i - (j+2)*(j+1)/2
     else:
         err_str = "Error in processing matrix indices; i and j must be integers"\

testsuite/AUTHORS

@@ -73,7 +73,7 @@ Chronological list of authors
 
 2017
   - Utkarsh Bansal
-
+  - Vedant Rathore
 
 External code
 -------------

testsuite/CHANGELOG

@@ -14,7 +14,7 @@ and https://github.com/MDAnalysis/mdanalysis/wiki/UnitTests
 
 ------------------------------------------------------------------------------
 ??/??/16 jbarnoud, orbeckst, fiona-naughton, manuel.nuno.melo, richardjgowers
-         tyler.je.reddy, utkbansal
+         tyler.je.reddy, utkbansal, vedantrathore
 
   * 0.16
     - added two unit tests for MDAnalysis.analysis.polymer

testsuite/MDAnalysisTests/analysis/test_psa.py

@@ -26,7 +26,7 @@
 
 from numpy.testing import (TestCase, dec, assert_array_less,
                            assert_array_almost_equal, assert_,
-                           assert_almost_equal)
+                           assert_almost_equal, assert_equal)
 import numpy as np
 
 from MDAnalysisTests.datafiles import PSF, DCD, DCD2
@@ -93,6 +93,15 @@ def test_dendrogram_produced(self):
         err_msg = "Dendrogram dictionary object was not produced"
         assert_(type(self.plot_data[1]) is dict, err_msg)
 
+    def test_dist_mat_to_vec_i_less_j(self):
+        """Test the index of corresponding distance vector is correct if i < j"""
+        err_msg = "dist_mat_to_vec function returning wrong values"
+        assert_equal(PSA.dist_mat_to_vec(5, 3, 4), 9, err_msg)
+
+    def test_dist_mat_to_vec_i_greater_j(self):
+        """Test the index of corresponding distance vector is correct if i > j"""
+        err_msg = "dist_mat_to_vec function returning wrong values"
+        assert_equal(PSA.dist_mat_to_vec(5, 4, 3), 9, err_msg)
 
 class TestPSAExceptions(TestCase):
     '''Tests for exceptions that should be raised
@@ -108,13 +117,60 @@ def test_get_path_metric_func_bad_key(self):
             self.fail('KeyError should be caught')
 
     def test_get_coord_axes_bad_dims(self):
-        '''Test that ValueError is raised when
+        """Test that ValueError is raised when
         numpy array with incorrect dimensions
-        is fed to get_coord_axes().'''
+        is fed to get_coord_axes()."""
 
         with self.assertRaises(ValueError):
             PSA.get_coord_axes(np.zeros((5,5,5,5)))
 
+    def test_dist_mat_to_vec_func_out_of_bounds(self):
+        """Test that ValueError is raised when i or j or both are
+        out of bounds of N"""
+
+        # Check if i is out of bounds of N
+        with self.assertRaises(ValueError):
+            PSA.dist_mat_to_vec(5, 6, 4)
+
+        # Check if j is out of bounds of N
+        with self.assertRaises(ValueError):
+            PSA.dist_mat_to_vec(5, 4, 6)
+
+        # Check if both i and j are out of bounds of N
+        with self.assertRaises(ValueError):
+            PSA.dist_mat_to_vec(5, 6, 7)
+
+        # Check if i is negative
+        with self.assertRaises(ValueError):
+            PSA.dist_mat_to_vec(5, -1, 2)
+
+        # Check if j is negative
+        with self.assertRaises(ValueError):
+            PSA.dist_mat_to_vec(5, 1, -2)
+
+        # Check if N is less than 2
+        with self.assertRaises(ValueError):
+            PSA.dist_mat_to_vec(1, 0, 0)
+
+    def test_dist_mat_to_vec_func_i_equals_j(self):
+        """Test that ValueError is raised when i == j or i,j == N"""
+
+        with self.assertRaises(ValueError):
+            PSA.dist_mat_to_vec(5, 4, 4)
+
+        with self.assertRaises(ValueError):
+            PSA.dist_mat_to_vec(4, 6, 4)
+
+    def test_dist_mat_to_vec_func_bad_integers(self):
+        """Test that ValueError is raised when i or j are
+        not Integers"""
+
+        with self.assertRaises(ValueError):
+            PSA.dist_mat_to_vec(5, '6', '7')
+
+        with self.assertRaises(ValueError):
+            PSA.dist_mat_to_vec(5, float(6), 7)
+
 class _BaseHausdorffDistance(TestCase):
     '''Base Class setup and unit tests
     for various Hausdorff distance