Merge pull request #49 from neutrons/security-linting

Add security linting

.github/workflows/actions.yml

@@ -37,7 +37,9 @@ jobs:
       - name: Lint with Tox
         if: ${{ matrix.python-version == env.PYTHON_MAIN_VERSION }}
         shell: bash -l {0}
-        run: tox -e lint
+        run: |
+          tox -e lint
+          tox -e lint-security
 
       - name: Test with Tox
         shell: bash -l {0}

Pipfile

@@ -11,6 +11,7 @@ autopep8 = "*"
 tox = "*"
 notebook = "*"
 sphinx-rtd-theme = "*"
+bandit = "*"
 
 [packages]
 numpy = "*"

pystog/cli.py

@@ -11,13 +11,10 @@
 import json
 
 from pystog import StoG
+from pystog.stog import NoInputFilesException
 from pystog.io import get_cli_parser, parse_cli_args
 
 
-class NoInputFilesException(Exception):
-    """Exception when no files are given to process"""
-
-
 def pystog_cli(kwargs=None):
     """
     Main entry point for PyStoG CLI tool
@@ -26,8 +23,9 @@ def pystog_cli(kwargs=None):
                    If None, parsed from command line via get_cli_parser
     :type kwargs: dict
     """
+    parser = get_cli_parser()
+
     if not kwargs:
-        parser = get_cli_parser()
         args = parser.parse_args()
         if args.json:
             print("loading config from '%s'" % args.json)

pystog/stog.py

@@ -17,6 +17,10 @@
 from pystog.fourier_filter import FourierFilter
 
 
+class NoInputFilesException(Exception):
+    """Exception when no files are given to process"""
+
+
 class StoG(object):
     """
     The StoG class is used to put together
@@ -543,8 +547,8 @@ def reciprocal_individuals(self):
         return self.__reciprocal_individuals
 
     @reciprocal_individuals.setter
-    def reciprocal_individuals(self, df):
-        self.__reciprocal_individuals = df
+    def reciprocal_individuals(self, individuals):
+        self.__reciprocal_individuals = individuals
 
     @property
     def sq_individuals(self):
@@ -562,8 +566,8 @@ def sq_individuals(self):
         return self.__sq_individuals
 
     @sq_individuals.setter
-    def sq_individuals(self, df):
-        self.__sq_individuals = df
+    def sq_individuals(self, individuals):
+        self.__sq_individuals = individuals
 
     @property
     def sq_master(self):
@@ -580,8 +584,8 @@ def sq_master(self):
         return self.__sq_master
 
     @sq_master.setter
-    def sq_master(self, df):
-        self.__sq_master = df
+    def sq_master(self, sq):
+        self.__sq_master = sq
 
     @property
     def gr_master(self):
@@ -598,8 +602,8 @@ def gr_master(self):
         return self.__gr_master
 
     @gr_master.setter
-    def gr_master(self, df):
-        self.__gr_master = df
+    def gr_master(self, gr):
+        self.__gr_master = gr
 
     @property
     def q_master(self):
@@ -616,8 +620,8 @@ def q_master(self):
         return self.__q_master
 
     @q_master.setter
-    def q_master(self, df):
-        self.__q_master = df
+    def q_master(self, q):
+        self.__q_master = q
 
     @property
     def r_master(self):
@@ -634,8 +638,8 @@ def r_master(self):
         return self.__r_master
 
     @r_master.setter
-    def r_master(self, df):
-        self.__r_master = df
+    def r_master(self, r):
+        self.__r_master = r
 
     @property
     def real_space_function(self):
@@ -801,9 +805,11 @@ def read_all_data(self, **kwargs):
         the **sq_individuals** numpy storage array in **add_dataset** method
         via **read_dataset** method.
         """
-        assert self.files is not None
-        assert len(self.files) != 0
+        # Check that we have files to operate on
+        if not self.files:
+            raise NoInputFilesException("No input files given in arguments")
 
+        # Read in all the data files
         for i, file_info in enumerate(self.files):
             self.read_dataset(file_info, **kwargs)
 
@@ -824,10 +830,12 @@ def read_dataset(
         :param info: Dict with information for dataset
                      (filename, manipulations, etc.)
         :type info: dict
-        :param xcol: The column in the data file that contains the X-axis
+        :param xcol: Column in data file for X-axis
         :type xcol: int
-        :param ycol: The column in the data file that contains the Y-axis
+        :param ycol: Column in data file for Y-axis
         :type ycol: int
+        :param dycol: Column in data file for Y uncertainty
+        :type dycol: int
         :param sep: Separator for the file used by numpy.loadtxt
         :type sep: raw string
         :param skiprows: Number of rows to skip. Passed to numpy.loadtxt
@@ -905,10 +913,11 @@ def add_dataset(
                 xoffset = info['X']['Offset']
 
         if adjusting:
-            x, y, dy = self._apply_scales_and_offset(x, y, dy=dy,
-                                                     yscale=yscale,
-                                                     yoffset=yoffset,
-                                                     xoffset=xoffset)
+            x, y, dy = self.apply_scales_and_offset(
+                x, y, dy=dy,
+                yscale=yscale,
+                yoffset=yoffset,
+                xoffset=xoffset)
 
         # Save overal x-axis min and max
         self.xmin = min(self.xmin, xmin)
@@ -953,8 +962,8 @@ def add_dataset(
         array_seq = (self.sq_individuals, np.stack((x, y, dy)))
         self.sq_individuals = np.concatenate(array_seq, axis=1)
 
-    def _apply_scales_and_offset(
-            self,
+    @staticmethod
+    def apply_scales_and_offset(
             x,
             y,
             dy=None,
@@ -977,42 +986,14 @@ def _apply_scales_and_offset(
         :return: X and Y vectors after scales and offsets applied
         :rtype: numpy.array pair
         """
-        y = self._scale(y, yscale)
-        y = self._offset(y, yoffset)
-        x = self._offset(x, xoffset)
+        y = y * yscale
+        y = y + yoffset
+        x = x + xoffset
         if dy is None:
             dy = np.zeros_like(y)
-        dy = self._scale(dy, yscale)
+        dy = dy * yscale
         return x, y, dy
 
-    def _offset(self, data, offset):
-        """
-        Applies offset to data
-
-        :param data: Input data
-        :type data: numpy.array or list
-        :param offset: Offset to apply to data
-        :type offset: float
-        :return: Data with offset applied
-        :rtype: numpy.array
-        """
-        data = data + offset
-        return data
-
-    def _scale(self, data, scale):
-        """
-        Applies scale to data
-
-        :param data: Input data
-        :type data: numpy.array or list
-        :param offset: Scale to apply to data
-        :type offset: float
-        :return: Data with scale applied
-        :rtype: numpy.array
-        """
-        data = scale * data
-        return data
-
     def merge_data(self):
         """
         Merges the reciprocal space data stored in the
@@ -1095,7 +1076,7 @@ def merge_data(self):
         sq = data_merged[1]
         dsq = data_merged[2]
 
-        q, sq, dsq = self._apply_scales_and_offset(
+        q, sq, dsq = self.apply_scales_and_offset(
             q, sq,
             yscale=self.merged_opts['Y']['Scale'],
             yoffset=self.merged_opts['Y']['Offset'],

requirements-dev.txt

@@ -1,3 +1,4 @@
+bandit
 mock
 pytest
 flake8

tests/test_cli.py

@@ -0,0 +1,8 @@
+import pytest
+from pystog.cli import pystog_cli
+from pystog.stog import NoInputFilesException
+
+
+def test_pystog_cli_no_files_exception():
+    with pytest.raises(NoInputFilesException):
+        pystog_cli({"cat": "meow"})

tests/test_converter.py

@@ -280,10 +280,18 @@ def F_to_S(self):
             atol=self.atol)
         assert_allclose(dsq, np.ones_like(self.q) / self.q)
 
+    def F_to_S_with_no_dfq(self):
+        sq, dsq = self.converter.F_to_S(self.q, self.fq, **self.kwargs)
+        assert_allclose(
+            sq[self.first:self.last],
+            self.sq_target,
+            rtol=self.rtol,
+            atol=self.atol)
+        assert_allclose(dsq, np.zeros_like(dsq))
+
     def F_to_FK(self):
-        fq_keen, dfq_keen = self.converter.F_to_FK(self.q, self.fq,
-                                                   np.ones_like(self.q),
-                                                   **self.kwargs)
+        fq_keen, dfq_keen = self.converter.F_to_FK(
+            self.q, self.fq, np.ones_like(self.q), **self.kwargs)
         assert_allclose(
             fq_keen[self.first:self.last],
             self.fq_keen_target,
@@ -293,6 +301,16 @@ def F_to_FK(self):
             dfq_keen,
             np.ones_like(self.q) * self.kwargs['<b_coh>^2'] / self.q)
 
+    def F_to_FK_with_no_dfq(self):
+        fq_keen, dfq_keen = self.converter.F_to_FK(
+            self.q, self.fq, **self.kwargs)
+        assert_allclose(
+            fq_keen[self.first:self.last],
+            self.fq_keen_target,
+            rtol=self.rtol,
+            atol=self.atol)
+        assert_allclose(dfq_keen, np.zeros_like(dfq_keen))
+
     def F_to_DCS(self):
         dcs, ddcs = self.converter.F_to_DCS(self.q, self.fq,
                                             np.ones_like(self.q),
@@ -395,9 +413,15 @@ def test_S_to_DCS(self):
     def test_F_to_S(self):
         self.F_to_S()
 
+    def test_F_to_S_with_no_dfq(self):
+        self.F_to_S_with_no_dfq()
+
     def test_F_to_FK(self):
         self.F_to_FK()
 
+    def test_F_to_FK_with_no_dfq(self):
+        self.F_to_FK_with_no_dfq()
+
     def test_F_to_DCS(self):
         self.F_to_DCS()
 
@@ -438,9 +462,15 @@ def test_S_to_DCS(self):
     def test_F_to_S(self):
         self.F_to_S()
 
+    def test_F_to_S_with_no_dfq(self):
+        self.F_to_S_with_no_dfq()
+
     def test_F_to_FK(self):
         self.F_to_FK()
 
+    def test_F_to_FK_with_no_dfq(self):
+        self.F_to_FK_with_no_dfq()
+
     def test_F_to_DCS(self):
         self.F_to_DCS()