Repository: spacetelescope/asdf (python)

Issue:
Add option to skip validation
Reasons for doing this will probably be related to performance. I'm inclined to say that skipping validation should result in a loud warning so that users do not get into the habit of doing this all the time.

Pull request description:
Add flag to skip validation on read
This PR adds a flag to `asdf.open` and `fits_embed.AsdfInFits.open` that disables validation on read, for situations when the file is already known to be valid and the user wishes to avoid the performance penalty of an unnecessary validation pass.

Resolves #565 

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 0d62a199..5ddfb42b 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -4,6 +4,9 @@
 - Improve read performance by skipping unnecessary rebuild
   of tagged tree. [#787]
 
+- Add option to ``asdf.open`` and ``fits_embed.AsdfInFits.open``
+  that disables validation on read. [#792]
+
 2.6.1 (unreleased)
 ------------------
 
diff --git a/asdf/asdf.py b/asdf/asdf.py
index 0c8c38de..1319ed84 100644
--- a/asdf/asdf.py
+++ b/asdf/asdf.py
@@ -610,7 +610,8 @@ class AsdfFile(versioning.VersionedMixin):
                    _get_yaml_content=False,
                    _force_raw_types=False,
                    strict_extension_check=False,
-                   ignore_missing_extensions=False):
+                   ignore_missing_extensions=False,
+                   validate_on_read=True):
         """Attempt to populate AsdfFile data from file-like object"""
 
         if strict_extension_check and ignore_missing_extensions:
@@ -670,11 +671,12 @@ class AsdfFile(versioning.VersionedMixin):
         if not do_not_fill_defaults:
             schema.fill_defaults(tree, self, reading=True)
 
-        try:
-            self._validate(tree, reading=True)
-        except ValidationError:
-            self.close()
-            raise
+        if validate_on_read:
+            try:
+                self._validate(tree, reading=True)
+            except ValidationError:
+                self.close()
+                raise
 
         tree = yamlutil.tagged_tree_to_custom_tree(tree, self, _force_raw_types)
 
@@ -693,7 +695,8 @@ class AsdfFile(versioning.VersionedMixin):
                    _get_yaml_content=False,
                    _force_raw_types=False,
                    strict_extension_check=False,
-                   ignore_missing_extensions=False):
+                   ignore_missing_extensions=False,
+                   validate_on_read=True):
         """Attempt to open file-like object as either AsdfFile or AsdfInFits"""
         if not is_asdf_file(fd):
             try:
@@ -708,7 +711,8 @@ class AsdfFile(versioning.VersionedMixin):
                             strict_extension_check=strict_extension_check,
                             ignore_missing_extensions=ignore_missing_extensions,
                             ignore_unrecognized_tag=self._ignore_unrecognized_tag,
-                            _extension_metadata=self._extension_metadata)
+                            _extension_metadata=self._extension_metadata,
+                            validate_on_read=validate_on_read)
             except ValueError:
                 raise ValueError(
                     "Input object does not appear to be an ASDF file or a FITS with " +
@@ -724,7 +728,8 @@ class AsdfFile(versioning.VersionedMixin):
                 _get_yaml_content=_get_yaml_content,
                 _force_raw_types=_force_raw_types,
                 strict_extension_check=strict_extension_check,
-                ignore_missing_extensions=ignore_missing_extensions)
+                ignore_missing_extensions=ignore_missing_extensions,
+                validate_on_read=validate_on_read)
 
     @classmethod
     def open(cls, fd, uri=None, mode='r',
@@ -1352,7 +1357,8 @@ def open_asdf(fd, uri=None, mode=None, validate_checksums=False,
               ignore_version_mismatch=True, ignore_unrecognized_tag=False,
               _force_raw_types=False, copy_arrays=False, lazy_load=True,
               custom_schema=None, strict_extension_check=False,
-              ignore_missing_extensions=False, _compat=False):
+              ignore_missing_extensions=False, validate_on_read=True,
+              _compat=False):
     """
     Open an existing ASDF file.
 
@@ -1421,6 +1427,10 @@ def open_asdf(fd, uri=None, mode=None, validate_checksums=False,
         contains metadata about extensions that are not available. Defaults
         to `False`.
 
+    validate_on_read : bool, optional
+        When `True`, validate the newly opened file against tag and custom
+        schemas.  Recommended unless the file is already known to be valid.
+
     Returns
     -------
     asdffile : AsdfFile
@@ -1447,7 +1457,8 @@ def open_asdf(fd, uri=None, mode=None, validate_checksums=False,
         do_not_fill_defaults=do_not_fill_defaults,
         _force_raw_types=_force_raw_types,
         strict_extension_check=strict_extension_check,
-        ignore_missing_extensions=ignore_missing_extensions)
+        ignore_missing_extensions=ignore_missing_extensions,
+        validate_on_read=validate_on_read)
 
 
 def is_asdf_file(fd):
diff --git a/asdf/fits_embed.py b/asdf/fits_embed.py
index d330b88e..49ac7ea3 100644
--- a/asdf/fits_embed.py
+++ b/asdf/fits_embed.py
@@ -169,7 +169,8 @@ class AsdfInFits(asdf.AsdfFile):
     @classmethod
     def open(cls, fd, uri=None, validate_checksums=False, extensions=None,
              ignore_version_mismatch=True, ignore_unrecognized_tag=False,
-             strict_extension_check=False, ignore_missing_extensions=False):
+             strict_extension_check=False, ignore_missing_extensions=False,
+             validate_on_read=True):
         """Creates a new AsdfInFits object based on given input data
 
         Parameters
@@ -207,6 +208,10 @@ class AsdfInFits(asdf.AsdfFile):
             When `True`, do not raise warnings when a file is read that
             contains metadata about extensions that are not available. Defaults
             to `False`.
+
+        validate_on_read : bool, optional
+            When `True`, validate the newly opened file against tag and custom
+            schemas.  Recommended unless the file is already known to be valid.
         """
         return cls._open_impl(fd, uri=uri,
                        validate_checksums=validate_checksums,
@@ -214,13 +219,14 @@ class AsdfInFits(asdf.AsdfFile):
                        ignore_version_mismatch=ignore_version_mismatch,
                        ignore_unrecognized_tag=ignore_unrecognized_tag,
                        strict_extension_check=strict_extension_check,
-                       ignore_missing_extensions=ignore_missing_extensions)
+                       ignore_missing_extensions=ignore_missing_extensions,
+                       validate_on_read=validate_on_read)
 
     @classmethod
     def _open_impl(cls, fd, uri=None, validate_checksums=False, extensions=None,
              ignore_version_mismatch=True, ignore_unrecognized_tag=False,
              strict_extension_check=False, _extension_metadata=None,
-             ignore_missing_extensions=False):
+             ignore_missing_extensions=False, validate_on_read=True):
 
         close_hdulist = False
         if isinstance(fd, fits.hdu.hdulist.HDUList):
@@ -256,7 +262,8 @@ class AsdfInFits(asdf.AsdfFile):
             return cls._open_asdf(self, buff, uri=uri, mode='r',
                               validate_checksums=validate_checksums,
                               strict_extension_check=strict_extension_check,
-                              ignore_missing_extensions=ignore_missing_extensions)
+                              ignore_missing_extensions=ignore_missing_extensions,
+                              validate_on_read=validate_on_read)
         except RuntimeError:
             self.close()
             raise


Test patch:
diff --git a/asdf/tests/test_api.py b/asdf/tests/test_api.py
index d9469f74..2d693332 100644
--- a/asdf/tests/test_api.py
+++ b/asdf/tests/test_api.py
@@ -10,6 +10,8 @@ from astropy.modeling import models
 
 import pytest
 
+from jsonschema.exceptions import ValidationError
+
 import asdf
 from asdf import treeutil
 from asdf import extension
@@ -17,7 +19,7 @@ from asdf import resolver
 from asdf import schema
 from asdf import versioning
 from asdf.exceptions import AsdfDeprecationWarning
-from .helpers import assert_tree_match, assert_roundtrip_tree, display_warnings
+from .helpers import assert_tree_match, assert_roundtrip_tree, display_warnings, yaml_to_asdf
 
 
 def test_get_data_from_closed_file(tmpdir):
@@ -82,6 +84,25 @@ def test_open_readonly(tmpdir):
             pass
 
 
+def test_open_validate_on_read(tmpdir):
+    content = """
+invalid_software: !core/software-1.0.0
+  name: Minesweeper
+  version: 3
+"""
+    buff = yaml_to_asdf(content)
+
+    with pytest.raises(ValidationError):
+        with asdf.open(buff, validate_on_read=True):
+            pass
+
+    buff.seek(0)
+
+    with asdf.open(buff, validate_on_read=False) as af:
+        assert af["invalid_software"]["name"] == "Minesweeper"
+        assert af["invalid_software"]["version"] == 3
+
+
 def test_atomic_write(tmpdir, small_tree):
     tmpfile = os.path.join(str(tmpdir), 'test.asdf')
 
diff --git a/asdf/tests/test_fits_embed.py b/asdf/tests/test_fits_embed.py
index 9e2e73f7..bebc708a 100644
--- a/asdf/tests/test_fits_embed.py
+++ b/asdf/tests/test_fits_embed.py
@@ -14,11 +14,13 @@ astropy = pytest.importorskip('astropy')
 from astropy.io import fits
 from astropy.table import Table
 
+from jsonschema.exceptions import ValidationError
+
 import asdf
 from asdf import fits_embed
 from asdf import open as asdf_open
 
-from .helpers import assert_tree_match, display_warnings, get_test_data_path
+from .helpers import assert_tree_match, display_warnings, get_test_data_path, yaml_to_asdf
 
 
 def create_asdf_in_fits():
@@ -284,6 +286,33 @@ def test_asdf_open(tmpdir):
         with asdf_open(hdulist) as ff:
             compare_asdfs(asdf_in_fits, ff)
 
+def test_validate_on_read(tmpdir):
+    tmpfile = str(tmpdir.join('invalid.fits'))
+
+    content = """
+invalid_software: !core/software-1.0.0
+  name: Minesweeper
+  version: 3
+"""
+    buff = yaml_to_asdf(content)
+    hdul = fits.HDUList()
+    data = np.array(buff.getbuffer(), dtype=np.uint8)[None, :]
+    fmt = '{}B'.format(len(data[0]))
+    column = fits.Column(array=data, format=fmt, name='ASDF_METADATA')
+    hdu = fits.BinTableHDU.from_columns([column], name='ASDF')
+    hdul.append(hdu)
+    hdul.writeto(tmpfile)
+
+    for open_method in [asdf.open, fits_embed.AsdfInFits.open]:
+        with pytest.raises(ValidationError):
+            with open_method(tmpfile, validate_on_read=True):
+                pass
+
+        with open_method(tmpfile, validate_on_read=False) as af:
+            assert af["invalid_software"]["name"] == "Minesweeper"
+            assert af["invalid_software"]["version"] == 3
+
+
 def test_open_gzipped():
     testfile = get_test_data_path('asdf.fits.gz')