Merge pull request #68 from pysat/tst/codeclimate

STY/TST: reduce code duplication, codeclimate updates

.codeclimate.yml

@@ -2,16 +2,13 @@
 plugins:
   radon:
     enabled: true
-  pep8:
-    enabled: true
   sonar-python:
     enabled: true
 ratings:
   paths:
   - "**.py"
 exclude_paths:
   - ".github/**/*"
-  - "pysatMissions/tests/**/*"
   - "docs/**/*"
   - "setup.py"
   - ".*"

CHANGELOG.md

@@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
   * Improve docstrings throughout
 * Testing
   * Add style check for docstrings
+  * Improve checks in codeclimate
 
 ## [0.2.2] - 2021-06-18
 * Migrate pyglow interface to pysatIncubator

pysatMissions/instruments/methods/orbits.py

@@ -21,9 +21,10 @@ def _check_orbital_params(kwargs=None):
     tles = ['TLE1', 'TLE2']
     errmsg = 'Insufficient kwargs.  Kwarg group requires {:}'
     for group in [tles, keplerians]:
-        if any(v in elements for v in group):
-            if not all(v in elements for v in group):
-                raise KeyError(errmsg.format(', '.join(group)))
+        bools = [v in elements for v in group]
+        # Check if group is incomplete.
+        if any(bools) and not all(bools):
+            raise KeyError(errmsg.format(', '.join(group)))
     if all(v in elements for v in tles) and all(v in elements for v in keplerians):
         warnings.warn(' '.join(['Cannot use both Keplerians and TLEs.',
                                 'Defaulting to Keplerians.']))

pysatMissions/instruments/missions_sgp4.py

@@ -205,9 +205,10 @@ def load(fnames, tag=None, inst_id=None, TLE1=None, TLE2=None,
     err_code, position, velocity = satellite.sgp4_array(jd, fr)
 
     # Check all propagated values for errors in propagation
-    for i in range(1, 7):
-        if np.any(err_code == i):
-            raise ValueError(sapi.SGP4_ERRORS[i])
+    errors = np.unique(err_code[err_code > 0])
+    if len(errors) > 0:
+        # Raise highest priority error.
+        raise ValueError(sapi.SGP4_ERRORS[errors[0]])
 
     # Add ECEF values to instrument.
 

pysatMissions/tests/test_inst_methods_orbits.py

@@ -9,18 +9,27 @@ class TestBasics(object):
 
     def setup(self):
         """Create a clean testing setup before each method."""
+
         self.orbit = {'inclination': 13, 'apogee': 850, 'perigee': 400,
                       'eccentricity': 0.032160315599897085,
                       'mean_motion': 0.06474416985702029}
         return
 
     def teardown(self):
         """Clean up test environment after each method."""
+
         del self.orbit
         return
 
+    def eval_output(self, value, label):
+        """Evaluate output value from calculations."""
+
+        assert abs(value - self.orbit[label]) / value < 1.e-6
+        return
+
     def test_convert_wrong_planet(self):
         """Test conversion routines with an unsupported planet."""
+
         with pytest.raises(KeyError) as kerr:
             mm_orbits.convert_to_keplerian(self.orbit['perigee'],
                                            self.orbit['apogee'], 'Hwae')
@@ -29,16 +38,18 @@ def test_convert_wrong_planet(self):
 
     def test_convert_to_keplerian(self):
         """Test conversion to keplerian elements."""
+
         ecc, mm, = mm_orbits.convert_to_keplerian(self.orbit['perigee'],
                                                   self.orbit['apogee'],)
-        assert abs(ecc - self.orbit['eccentricity']) / ecc < 1.e-6
-        assert abs(mm - self.orbit['mean_motion']) / mm < 1.e-6
+        self.eval_output(ecc, 'eccentricity')
+        self.eval_output(mm, 'mean_motion')
         return
 
     def test_convert_from_keplerian(self):
         """Test conversion from keplerian elements."""
+
         per, apo, = mm_orbits.convert_from_keplerian(self.orbit['eccentricity'],
                                                      self.orbit['mean_motion'],)
-        assert abs(per - self.orbit['perigee']) / per < 1.e-6
-        assert abs(apo - self.orbit['apogee']) / apo < 1.e-6
+        self.eval_output(per, 'perigee')
+        self.eval_output(apo, 'apogee')
         return

pysatMissions/tests/test_instruments.py

@@ -23,8 +23,6 @@
 from pysat.utils import generate_instrument_list
 
 
-saved_path = pysat.params['data_dirs']
-
 # Developers for instrument libraries should update the following line to
 # point to their own subpackage location
 # e.g.,

pysatMissions/tests/test_methods_magcoord.py

@@ -3,6 +3,9 @@
 
 import datetime as dt
 import numpy as np
+
+import pytest
+
 import pysat
 import pysatMissions.methods.magcoord as mm_magcoord
 
@@ -12,43 +15,43 @@ class TestBasics(object):
 
     def setup(self):
         """Create a clean testing setup before each method."""
+
         self.testInst = pysat.Instrument(platform='pysat', name='testing',
                                          num_samples=100, clean_level='clean')
+        self.kwargs = {'glat_label': 'latitude',
+                       'glong_label': 'longitude',
+                       'alt_label': 'altitude'}
+        self.reftime = dt.datetime(2009, 1, 1)
         return
 
     def teardown(self):
         """Clean up test environment after each method."""
-        del self
+
+        del self.testInst, self.kwargs, self.reftime
         return
 
-    def test_add_aacgm_coordinates(self):
-        """Test adding thermal plasma data to test inst."""
-        self.testInst.custom_attach(mm_magcoord.add_aacgm_coordinates,
-                                    kwargs={'glat_label': 'latitude',
-                                            'glong_label': 'longitude',
-                                            'alt_label': 'altitude'})
-        self.testInst.load(date=dt.datetime(2009, 1, 1))
-        targets = ['aacgm_lat', 'aacgm_long', 'aacgm_mlt']
+    def eval_targets(self, targets):
+        """Evaluate addition of new data targets to instrument."""
+
         for target in targets:
-            # Check if data is added
-            assert target in self.testInst.data.keys()
-            assert not np.isnan(self.testInst[target]).any()
-            # Check if metadata is added
-            assert target in self.testInst.meta.data.index
+            assert target in self.testInst.data.keys(), \
+                "{:s} not found in data".format(target)
+            assert not np.isnan(self.testInst[target]).any(), \
+                "NaN values found in {:s}".format(target)
+            assert target in self.testInst.meta.data.index, \
+                "{:s} not found in metadata".format(target)
         return
 
-    def test_add_quasi_dipole_coordinates(self):
+    @pytest.mark.parametrize("func,targets",
+                             [('add_aacgm_coordinates',
+                               ['aacgm_lat', 'aacgm_long', 'aacgm_mlt']),
+                              ('add_quasi_dipole_coordinates',
+                               ['qd_lat', 'qd_long', 'mlt'])])
+    def test_add_coordinates(self, func, targets):
         """Test adding thermal plasma data to test inst."""
-        self.testInst.custom_attach(mm_magcoord.add_quasi_dipole_coordinates,
-                                    kwargs={'glat_label': 'latitude',
-                                            'glong_label': 'longitude',
-                                            'alt_label': 'altitude'})
-        self.testInst.load(date=dt.datetime(2009, 1, 1))
-        targets = ['qd_lat', 'qd_long', 'mlt']
-        for target in targets:
-            # Check if data is added
-            assert target in self.testInst.data.keys()
-            assert not np.isnan(self.testInst[target]).any()
-            # Check if metadata is added
-            assert target in self.testInst.meta.data.index
+
+        self.testInst.custom_attach(getattr(mm_magcoord, func),
+                                    kwargs=self.kwargs)
+        self.testInst.load(date=self.reftime)
+        self.eval_targets(targets)
         return

pysatMissions/tests/test_methods_spacecraft.py

@@ -40,29 +40,35 @@ def setup(self):
         self.testInst = pysat.Instrument(platform='pysat', name='testing',
                                          num_samples=9, clean_level='clean')
         self.testInst.custom_attach(add_eci)
+        self.reftime = dt.datetime(2009, 1, 1)
         return
 
     def teardown(self):
         """Clean up test environment after tests."""
 
-        del self
+        del self.testInst, self.reftime
+        return
+
+    def eval_targets(self, targets):
+        """Evaluate addition of new data targets to instrument."""
+
+        for target in targets:
+            assert target in self.testInst.data.keys(), \
+                "{:s} not found in data".format(target)
+            # By default, endpoints will be NaNs.  Ignore these.
+            assert not np.isnan(self.testInst[target][1:-1]).any(), \
+                "NaN values found in {:s}".format(target)
+            assert target in self.testInst.meta.data.index, \
+                "{:s} not found in metadata".format(target)
         return
 
     def test_calculate_ecef_velocity(self):
         """Test `calculate_ecef_velocity` helper function."""
 
         self.testInst.custom_attach(mm_sc.calculate_ecef_velocity)
-        self.testInst.load(date=dt.datetime(2009, 1, 1))
+        self.testInst.load(date=self.reftime)
         targets = ['velocity_ecef_x', 'velocity_ecef_y', 'velocity_ecef_z']
-        for target in targets:
-            # Check if data is added
-            assert target in self.testInst.data.keys()
-            assert not np.isnan(self.testInst[target][1:-1]).any()
-            # Endpoints should be NaN
-            assert np.isnan(self.testInst[target][0])
-            assert np.isnan(self.testInst[target][-1])
-            # Check if metadata is added
-            assert target in self.testInst.meta.data.index
+        self.eval_targets(targets)
         return
 
     def test_add_ram_pointing_sc_attitude_vectors(self):
@@ -71,19 +77,11 @@ def test_add_ram_pointing_sc_attitude_vectors(self):
         # TODO(#64): check if calculations are correct
         self.testInst.custom_attach(mm_sc.calculate_ecef_velocity)
         self.testInst.custom_attach(mm_sc.add_ram_pointing_sc_attitude_vectors)
-        self.testInst.load(date=dt.datetime(2009, 1, 1))
+        self.testInst.load(date=self.reftime)
         targets = ['sc_xhat_ecef_x', 'sc_xhat_ecef_y', 'sc_xhat_ecef_z',
                    'sc_yhat_ecef_x', 'sc_yhat_ecef_y', 'sc_yhat_ecef_z',
                    'sc_zhat_ecef_x', 'sc_zhat_ecef_y', 'sc_zhat_ecef_z']
-        for target in targets:
-            # Check if data is added
-            assert target in self.testInst.data.keys()
-            assert not np.isnan(self.testInst[target][1:-1]).any()
-            # Endpoints should be NaN
-            assert np.isnan(self.testInst[target][0])
-            assert np.isnan(self.testInst[target][-1])
-            # Check if metadata is added
-            assert target in self.testInst.meta.data.index
+        self.eval_targets(targets)
         return
 
     def test_project_ecef_vector_onto_sc(self):
@@ -95,15 +93,7 @@ def test_project_ecef_vector_onto_sc(self):
         self.testInst.custom_attach(add_fake_data)
         self.testInst.custom_attach(mm_sc.project_ecef_vector_onto_sc,
                                     args=['ax', 'ay', 'az', 'bx', 'by', 'bz'])
-        self.testInst.load(date=dt.datetime(2009, 1, 1))
+        self.testInst.load(date=self.reftime)
         targets = ['bx', 'by', 'bz']
-        for target in targets:
-            # Check if data is added
-            assert target in self.testInst.data.keys()
-            assert not np.isnan(self.testInst[target][1:-1]).any()
-            # Endpoints should be NaN
-            assert np.isnan(self.testInst[target][0])
-            assert np.isnan(self.testInst[target][-1])
-            # Check if metadata is added
-            assert target in self.testInst.meta.data.index
+        self.eval_targets(targets)
         return