CADC-9033 - edge cases exposed by CFHT. (#140)

* CADC-9033 - edge cases exposed by CFHT.

caom2/caom2/diff.py

@@ -86,6 +86,14 @@
 __all__ = ['get_differences']
 
 
+# a list of dict keys that are not checked as a result of a get_differences
+# call:
+# 'meta_producer' is ignored because it can cause a lot of noise in github
+#                 commits for not a lot of value
+#
+ignore_keys = ['meta_producer']
+
+
 def get_differences(expected, actual, parent=None):
     """
     Compare two entities. Provide a report if differences exist between the two
@@ -360,6 +368,13 @@ def _get_dict_differences(expected, actual, parent):
     """Reports on how two dictionaries are different."""
     report = []
     for expected_key, expected_value in expected.items():
+        found = False
+        for ignore_key in ignore_keys:
+            if expected_key == ignore_key:
+                actual.pop(expected_key)
+                found = True
+        if found:
+            continue
         if expected_key in actual:
             actual_value = actual[expected_key]
             if _is_composite_instance_type(actual_value):

caom2/caom2/tests/test_diffs.py

@@ -86,6 +86,8 @@ def test_get_differences(self):
             collection='test_collection',
             observation_id='test_observation_id',
             algorithm=observation.Algorithm('EXPOSURE'))
+        # meta_producer field is ignored by get_difference
+        expected_simple.meta_producer = 'meta_producer/0.1'
         report = diff.get_differences(expected_simple, expected_simple,
                                       'obs')
         self.assertTrue(report is None, repr(report))

caom2utils/caom2utils/caomvalidator.py

@@ -215,7 +215,7 @@ def _validate_keyword(name, keywords):
     if not keywords:
         return
     for keyword in keywords:
-        if keyword.find('|') != -1:
+        if keyword is not None and keyword.find('|') != -1:
             raise AssertionError(
                 'invalid {}: may not contain pipe (|)'.format(name))
 

caom2utils/caom2utils/fits2caom2.py

@@ -3772,7 +3772,7 @@ def _update_artifact_meta(uri, artifact, subject=None, connected=True):
             # because the metadata is available long before the data
             # will be stored at CADC
             return
-    elif file_url.scheme == 'vos':
+    elif file_url.scheme in ['cadc', 'vos']:
         metadata = _get_vos_meta(subject, uri)
     elif file_url.scheme == 'file':
         if (file_url.path.endswith('.header') and subject is not None and

caom2utils/caom2utils/tests/test_fits2caom2.py

@@ -74,7 +74,8 @@
 from astropy.wcs import WCS as awcs
 from cadcutils import exceptions, net
 from caom2utils import FitsParser, WcsParser, main_app, update_blueprint
-from caom2utils import ObsBlueprint, GenericParser
+from caom2utils import ObsBlueprint, GenericParser, gen_proc
+from caom2utils import get_gen_proc_arg_parser
 from caom2utils.legacy import load_config
 from caom2utils.fits2caom2 import _visit, _load_plugin, _update_artifact_meta
 
@@ -114,10 +115,6 @@
 test_class_plugin_module = os.path.join(TESTDATA_DIR, 'test_plugin_class.py')
 non_conformant_plugin_module = os.path.join(TESTDATA_DIR, 'nonconformant.py')
 
-# to execute only one test in the file set this var to True and comment
-# out the skipif decorator of the test
-single_test = False
-
 
 class MyExitError(Exception):
     pass
@@ -147,7 +144,6 @@ class MyExitError(Exception):
 '''
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_augment_energy():
     bp = ObsBlueprint(energy_axis=1)
     test_fitsparser = FitsParser(sample_file_4axes, bp)
@@ -161,7 +157,6 @@ def test_augment_energy():
     assert result is None, repr(energy)
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_augment_artifact_energy_from_blueprint():
     test_blueprint = ObsBlueprint(energy_axis=1)
     test_blueprint.set('Chunk.energyAxis', 1)
@@ -204,7 +199,6 @@ def test_augment_artifact_energy_from_blueprint():
 '''
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_augment_polarization():
     test_fitsparser = FitsParser(sample_file_4axes,
                                  ObsBlueprint(polarization_axis=1))
@@ -219,7 +213,6 @@ def test_augment_polarization():
     assert result is None, result
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_augment_artifact_polarization_from_blueprint():
     test_blueprint = ObsBlueprint(polarization_axis=1)
     test_blueprint.set('Chunk.polarizationAxis', '1')
@@ -277,7 +270,6 @@ def test_augment_artifact_polarization_from_blueprint():
 '''
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_augment_artifact():
     test_blueprint = ObsBlueprint(position_axes=(1, 2))
     test_fitsparser = FitsParser(sample_file_4axes, test_blueprint)
@@ -296,7 +288,6 @@ def test_augment_artifact():
     assert result is None
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_augment_artifact_position_from_blueprint():
     test_blueprint = ObsBlueprint(position_axes=(1, 2))
     test_blueprint.set('Chunk.positionAxis1', '1')
@@ -356,7 +347,6 @@ def test_augment_artifact_position_from_blueprint():
 '''
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_augment_artifact_time():
     test_fitsparser = FitsParser(sample_file_time_axes,
                                  ObsBlueprint(time_axis=1))
@@ -375,7 +365,6 @@ def test_augment_artifact_time():
     assert result is None
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_augment_artifact_time_from_blueprint():
     test_blueprint = ObsBlueprint(time_axis=1)
     test_blueprint.set('Chunk.timeAxis', '1')
@@ -401,7 +390,6 @@ def test_augment_artifact_time_from_blueprint():
     assert result is None
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_get_wcs_values():
     w = get_test_wcs(sample_file_4axes)
     test_parser = WcsParser(get_test_header(sample_file_4axes)[0].header,
@@ -441,7 +429,6 @@ def _get_from_str_xml(string_xml, get_func, element_tag):
     return act_obj
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 @patch('sys.exit', Mock(side_effect=[MyExitError, MyExitError, MyExitError,
                                      MyExitError, MyExitError,
                                      MyExitError]))
@@ -581,7 +568,6 @@ def test_help():
 """
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_augment_observation():
     test_obs_blueprint = ObsBlueprint(position_axes=(1, 2))
     test_obs_blueprint.set('Observation.target.name', 'CGPS Mosaic MA1')
@@ -629,7 +615,6 @@ def test_augment_observation():
     assert diff_result is None
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_get_from_list():
     test_fitsparser = FitsParser(sample_file_4axes)
     test_fitsparser.blueprint = ObsBlueprint()
@@ -638,7 +623,6 @@ def test_get_from_list():
     assert result == ObservationIntentType.SCIENCE
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_update_fits_headers():
     # The rules for the values:
     # all upper case - a FITS keyword
@@ -808,14 +792,12 @@ def test_update_fits_headers():
      }}
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_load_config_overrides():
     # cool override file content
     result = load_config(override_file)
     assert result == TEST_OVERRIDES
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_chunk_naxis():
     hdr1 = fits.Header()
     test_blueprint = ObsBlueprint()
@@ -916,7 +898,6 @@ def test_chunk_naxis():
 """
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_file_scheme_uris():
     """ Tests that local files as URIs will be accepted and processed."""
 
@@ -974,7 +955,6 @@ def _get_obs(from_xml_string):
 """
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_generic_parser():
     """ Tests that GenericParser will be created."""
 
@@ -993,7 +973,6 @@ def test_generic_parser():
             assert result is None
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_visit():
     test_obs = _get_obs(EXPECTED_FILE_SCHEME_XML)
 
@@ -1117,7 +1096,6 @@ def test_visit():
 '''
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_augment_artifact_bounds_range_from_blueprint():
     test_blueprint = ObsBlueprint(energy_axis=1, time_axis=2,
                                   polarization_axis=3,
@@ -1197,7 +1175,6 @@ def test_augment_artifact_bounds_range_from_blueprint():
     assert result is None
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_visit_generic_parser():
     try:
         sys.argv = ['fits2caom2', '--local', 'fname', '--observation',
@@ -1215,7 +1192,6 @@ def test_visit_generic_parser():
         assert False, 'should not get here {}'.format(e)
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 @patch('caom2utils.fits2caom2.Client')
 def test_get_vos_headers(vos_mock):
     test_uri = 'vos://cadc.nrc.ca!vospace/CAOMworkshop/Examples/DAO/' \
@@ -1234,7 +1210,6 @@ def test_get_vos_headers(vos_mock):
         caom2utils.fits2caom2.get_cadc_headers = get_orig
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 @patch('caom2utils.fits2caom2.Client')
 def test_get_vos_meta(vos_mock):
     get_orig = caom2utils.get_vos_headers
@@ -1260,7 +1235,6 @@ def test_get_vos_meta(vos_mock):
         caom2utils.get_vos_headers = get_orig
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_generic_parser1():
     test_key = 'Plane.metaRelease'
     test_value = '2013-10-10'
@@ -1275,7 +1249,6 @@ def test_generic_parser1():
         'original value over-ridden'
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_get_external_headers():
     test_uri = 'http://localhost/obs23/collection/obsid-1'
     with patch('requests.Session.get') as session_get_mock:
@@ -1289,7 +1262,6 @@ def test_get_external_headers():
         assert session_get_mock.is_called_with(test_uri)
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_apply_blueprint():
     # test a Gemini case where there are two keywords, one each for
     # different instruments, and the default ends up getting set when the
@@ -1328,7 +1300,6 @@ def test_apply_blueprint():
         test_parser._headers[0]['IMAGESWV'], 'should not be set'
 
 
-@pytest.mark.skipif(single_test, reason='Single test mode')
 def test_update_artifact_meta_errors():
     test_uri = 'gemini:GEMINI/abc.jpg'
     test_artifact = Artifact(uri=test_uri,
@@ -1370,6 +1341,28 @@ def _mock(ig, nore):
         assert test_artifact.content_checksum is None, 'checksum'
 
 
+@patch('sys.stdout', new_callable=BytesIO)
+@patch('caom2utils.fits2caom2._augment')
+def test_gen_proc_failure(augment_mock, stdout_mock):
+    """ Tests that gen_proc can return -1."""
+
+    augment_mock.return_value = None  # return a broken Observation instance
+    fname = 'file://{}'.format(text_file)
+    sys.argv = ['fits2caom2', '--local', fname,
+                '--observation', 'test_collection_id',
+                'test_observation_id', '--lineage',
+                'test_product_id/ad:TEST/{}'.format(fname)]
+    test_args = get_gen_proc_arg_parser().parse_args()
+    test_blueprints = {'test_collection_id': ObsBlueprint()}
+    test_result = gen_proc(test_args, test_blueprints)
+    assert test_result == -1, 'expect failure'
+    if stdout_mock.getvalue():
+        expected = _get_obs(EXPECTED_GENERIC_PARSER_FILE_SCHEME_XML)
+        actual = _get_obs(stdout_mock.getvalue().decode('ascii'))
+        result = get_differences(expected, actual, 'Observation')
+        assert result is None
+
+
 def _get_headers(file_name, subject):
     x = """SIMPLE  =                    T / Written by IDL:  Fri Oct  6 01:48:35 2017
 BITPIX  =                  -32 / Bits per pixel

caom2utils/caom2utils/tests/test_obs_blueprint.py

@@ -100,6 +100,7 @@ def test_obs_blueprint():
     ob.configure_polarization_axis(axis=4)
     ob.configure_time_axis(axis=5)
     ob.configure_observable_axis(axis=6)
+    ob.configure_custom_axis(axis=7)
 
     # test that configuring something that's already configured doesn't break
     # anything
@@ -108,6 +109,7 @@ def test_obs_blueprint():
     ob.configure_polarization_axis(axis=4)
     ob.configure_time_axis(axis=5)
     ob.configure_observable_axis(axis=6)
+    ob.configure_custom_axis(axis=7)
 
     # set attribute
     ob.set('Observation.instrument.name', 'NIRI')
@@ -325,3 +327,8 @@ def test_has_chunk():
     ob.set('Chunk', '{ignore}', 1)
     assert ob.has_chunk(0)
     assert not ob.has_chunk(1)
+
+
+def test_ctor_failure():
+    with pytest.raises(ValueError):
+        test_subject = ObsBlueprint(position_axes=(1, 2, 3))

caom2utils/caom2utils/tests/test_wcsvalidator.py

@@ -109,11 +109,6 @@ def test_bad_temporalwcs(self):
                 self, InvalidWCSError, 'range.end not >= range.start'):
             wcsvalidator._validate_temporal_wcs(bad_temporal_wcs)
 
-        bad_temporal_wcs = TimeTestUtil.bad_delta()
-        with six.assertRaisesRegex(
-                self, InvalidWCSError, 'delta must be greater than 0.0'):
-            wcsvalidator._validate_temporal_wcs(bad_temporal_wcs)
-
 
 # CustomWCS validator tests
 @pytest.mark.skipif(single_test, reason='Single test mode')
@@ -290,18 +285,6 @@ def bad_cunit_wcs():
         badcunit.axis.axis.cunit = "foo"
         return badcunit
 
-    @staticmethod
-    def bad_delta():
-        axis_1d = wcs.CoordAxis1D(wcs.Axis("UTC", "d"))
-        temporal_wcs = chunk.TemporalWCS(axis_1d)
-
-        # delta < 0.0 is bad
-        ref_coord = wcs.RefCoord(float(1.0), float(2.0))
-        temporal_wcs.axis.function = CoordFunction1D(
-            int(100), -0.01, ref_coord)
-
-        return temporal_wcs
-
     @staticmethod
     def bad_range_wcs():
         px = float(0.5)

caom2utils/caom2utils/wcs_util.py

@@ -131,13 +131,18 @@ def function1d_to_interval(temporal_wcs, function_1d):
             # // units, like days, hours, minutes, seconds, and smaller
             # // since they are offsets from mjdref
 
+            # PD - 16-04-20
+            # technically there is nothing wrong with a WCS axis that
+            # decreases in coord values while increasing in pixel values
+            # (it's just a line with negative slope).
+            #
+            # the computation of the time bounds interval sorts out the
+            # min/max so it also doesn't care about the "direction"
+
             p1 = float(0.5)
             p2 = float(function_1d.naxis + 0.5)
             a = pix2val(function_1d, p1)
             b = pix2val(function_1d, p2)
-            if function_1d.delta < 0.0:
-                raise ValueError(
-                    '{} delta must be greater than 0.0'.format(function_1d))
 
             if temporal_wcs.mjdref is not None:
                 a += float(temporal_wcs.mjdref)

caom2utils/setup.cfg

@@ -34,7 +34,7 @@ edit_on_github = False
 github_project = opencadc/caom2tools
 install_requires = cadcdata>=1.2.3 caom2>=2.4 astropy>=2.0 spherical-geometry==1.2.11;python_version=="2.7" spherical-geometry>=1.2.17;python_version>="3.4" vos>=3.1.1
 # version should be PEP386 compatible (http://www.python.org/dev/peps/pep-0386)
-version = 1.5.1
+version = 1.6