Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement ability to hide warnings in VO table parsing #8715

Merged
merged 25 commits into from May 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
1e30869
Started implementing more fine-grained pedantic mode for VO table par…
astrofrog May 15, 2019
a71f8a9
Change default pedantic= value to 'ignore' so that no warnings are sh…
astrofrog May 16, 2019
0f0288e
Changed pedantic option to verify
astrofrog May 16, 2019
93818a2
Fix vo_warn behavior
astrofrog May 16, 2019
e206ad9
Added changelog entry
astrofrog May 16, 2019
0ed5907
Added unit test
astrofrog May 16, 2019
c5dd4f0
Test pedantic option
astrofrog May 16, 2019
e94daba
Apply suggestions from code review
astrofrog May 17, 2019
82283cd
Added more extensive tests of setting configuration items for verify …
astrofrog May 17, 2019
d790ae0
Fix parse() following removal of pedantic option
astrofrog May 17, 2019
86abb55
Updated package name in documentation for exceptions/warnings
astrofrog May 17, 2019
ee9701e
Remove pedantic from docstring
astrofrog May 17, 2019
51335e6
Added a .. versionchanged entry for the change from pedantic to verify
astrofrog May 17, 2019
ffd55f4
Fix logic in if statement
astrofrog May 17, 2019
13c67cc
Remove unused code
astrofrog May 17, 2019
2904429
Fix defaults for behavior in warning/exception-handling code
astrofrog May 17, 2019
fb3c873
Fix default value of verify in converters
astrofrog May 17, 2019
61f9e98
Apply suggestions from code review
astrofrog May 17, 2019
12ecba0
Refactor tests into class and split up tests
astrofrog May 17, 2019
d26a816
Change logic back for verify as booleans
astrofrog May 17, 2019
c9cf40e
Apply suggestions from code review
astrofrog May 22, 2019
d21679c
Fixed another instance of warn -> ignore
astrofrog May 22, 2019
ca8462a
Make sure we don't actually emit a deprecation warning yet when acces…
astrofrog May 23, 2019
c5add16
Don't specify verify='ignore' in tests since it is the default
astrofrog May 23, 2019
94826f0
Use raw string
astrofrog May 23, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGES.rst
Expand Up @@ -112,6 +112,11 @@ astropy.io.registry
astropy.io.votable
^^^^^^^^^^^^^^^^^^

- Changed ``pedantic`` argument to ``verify`` and change it to have three
astrofrog marked this conversation as resolved.
Show resolved Hide resolved
string-based options (``ignore``, ``warn``, and ``exception``) instead of just
being a boolean. In addition, changed default to ``ignore``, which means
that warnings will not be shown by default when loading VO tables. [#8715]

astropy.modeling
^^^^^^^^^^^^^^^^

Expand Down
11 changes: 7 additions & 4 deletions astropy/io/votable/__init__.py
Expand Up @@ -24,10 +24,13 @@ class Conf(_config.ConfigNamespace):
Configuration parameters for `astropy.io.votable`.
"""

pedantic = _config.ConfigItem(
False,
'When True, treat fixable violations of the VOTable spec as exceptions.',
aliases=['astropy.io.votable.table.pedantic'])
verify = _config.ConfigItem(
'ignore',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I look at this, I am actually not sure if alias would work if the config type has changed. When cfgtype is not provided, as in the case here, it is inferred from the default value. So with this change, its type is now string, where it used to be boolean. That is going to choke the validator when someone has an older astropy.cfg with, say, pedantic = True set. A possible solution is to explicitly state the cfgtype as 'option' and provide all the 5 possible options (3 new ones and 2 boolean values).

References:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason it still doesn't work right if I specify option with mixed types, and it always gets converted to a string. But that's ok, we can still be backward compatible by just checking for False and True as strings when using conf.verify. I've added handling of this as well as an extensive test of setting the configuration items (both verify and pedantic).

"Can be 'exception' (treat fixable violations of the VOTable spec as "
"exceptions), 'warn' (show warnings for VOTable spec violations), or "
"'ignore' (silently ignore VOTable spec violations)",
aliases=['astropy.io.votable.table.pedantic',
'astropy.io.votable.pedantic'])


conf = Conf()
12 changes: 10 additions & 2 deletions astropy/io/votable/connect.py
Expand Up @@ -44,7 +44,7 @@ def is_votable(origin, filepath, fileobj, *args, **kwargs):
return False


def read_table_votable(input, table_id=None, use_names_over_ids=False):
def read_table_votable(input, table_id=None, use_names_over_ids=False, verify=None):
"""
Read a Table object from an VO table file

Expand All @@ -68,9 +68,17 @@ def read_table_votable(input, table_id=None, use_names_over_ids=False):
are not guaranteed to be unique, this may cause some columns
to be renamed by appending numbers to the end. Otherwise
(default), use the ID attributes as the column names.

verify : {'ignore', 'warn', 'exception'}, optional
When ``'exception'``, raise an error when the file violates the spec,
otherwise either issue a warning (``'warn'``) or silently continue
(``'ignore'``). Warnings may be controlled using the standard Python
mechanisms. See the `warnings` module in the Python standard library
for more information. When not provided, uses the configuration setting
``astropy.io.votable.verify``, which defaults to ``'ignore'``.
"""
if not isinstance(input, (VOTableFile, VOTable)):
input = parse(input, table_id=table_id)
input = parse(input, table_id=table_id, verify=verify)

# Parse all table objects
table_id_mapping = dict()
Expand Down
8 changes: 4 additions & 4 deletions astropy/io/votable/converters.py
Expand Up @@ -319,7 +319,7 @@ def __init__(self, field, config=None, pos=None):
self.binoutput = self._binoutput_fixed
self._struct_format = ">{:d}s".format(self.arraysize)

if config.get('pedantic'):
if config.get('verify', 'ignore') == 'exception':
self.parse = self._ascii_parse
else:
self.parse = self._str_parse
Expand Down Expand Up @@ -439,7 +439,7 @@ def __init__(self, field, config=None, pos=None):
if config is None:
config = {}
Converter.__init__(self, field, config, pos)
if config.get('pedantic'):
if config.get('verify', 'ignore') == 'exception':
self._splitter = self._splitter_pedantic
else:
self._splitter = self._splitter_lax
Expand Down Expand Up @@ -578,7 +578,7 @@ def parse(self, value, config=None, pos=None):
parts = self._splitter(value, config, pos)
if len(parts) != self._items:
warn_or_raise(E02, E02, (self._items, len(parts)), config, pos)
if config.get('pedantic'):
if config.get('verify', 'ignore') == 'exception':
return self.parse_parts(parts, config, pos)
else:
if len(parts) == self._items:
Expand Down Expand Up @@ -698,7 +698,7 @@ def __init__(self, field, config=None, pos=None):
self._null_binoutput = self.binoutput(np.asarray(self.null), False)
self.filter_array = self._filter_null

if config.get('pedantic'):
if config.get('verify', 'ignore') == 'exception':
self.parse = self._parse_pedantic
else:
self.parse = self._parse_permissive
Expand Down
74 changes: 41 additions & 33 deletions astropy/io/votable/exceptions.py
Expand Up @@ -24,9 +24,9 @@

.. note::

This is a list of many of the fatal exceptions emitted by vo.table
This is a list of many of the fatal exceptions emitted by ``astropy.io.votable``
when the file does not conform to spec. Other exceptions may be
raised due to unforeseen cases or bugs in vo.table itself.
raised due to unforeseen cases or bugs in ``astropy.io.votable`` itself.

{exceptions}
"""
Expand Down Expand Up @@ -77,15 +77,19 @@ def _suppressed_warning(warning, config, stacklevel=2):
def warn_or_raise(warning_class, exception_class=None, args=(), config=None,
pos=None, stacklevel=1):
"""
Warn or raise an exception, depending on the pedantic setting.
Warn or raise an exception, depending on the verify setting.
"""
if config is None:
config = {}
if config.get('pedantic'):
# NOTE: the default here is deliberately warn rather than ignore, since
# one would expect that calling warn_or_raise without config should not
# silence the warnings.
config_value = config.get('verify', 'warn')
if config_value == 'exception':
if exception_class is None:
exception_class = warning_class
vo_raise(exception_class, args, config, pos)
else:
elif config_value == 'warn':
vo_warn(warning_class, args, config, pos, stacklevel=stacklevel+1)


Expand Down Expand Up @@ -122,8 +126,12 @@ def vo_warn(warning_class, args=(), config=None, pos=None, stacklevel=1):
"""
if config is None:
config = {}
warning = warning_class(args, config, pos)
_suppressed_warning(warning, config, stacklevel=stacklevel+1)
# NOTE: the default here is deliberately warn rather than ignore, since
# one would expect that calling warn_or_raise without config should not
# silence the warnings.
if config.get('verify', 'warn') != 'ignore':
warning = warning_class(args, config, pos)
_suppressed_warning(warning, config, stacklevel=stacklevel+1)


def warn_unknown_attrs(element, attrs, config, pos, good_attr=[], stacklevel=1):
Expand Down Expand Up @@ -249,10 +257,10 @@ class W01(VOTableSpecWarning):
encoded as multiple numbers separated by whitespace.

Many VOTable files in the wild use commas as a separator instead,
and ``vo.table`` supports this convention when not in
and ``astropy.io.votable`` supports this convention when not in
:ref:`pedantic-mode`.

``vo.table`` always outputs files using only spaces, regardless of
``astropy.io.votable`` always outputs files using only spaces, regardless of
how they were input.

**References**: `1.1
Expand Down Expand Up @@ -280,7 +288,7 @@ class W02(VOTableSpecWarning):

However, this is in conflict with the XML standard, which says
colons may not be used. VOTable 1.1's own schema does not allow a
colon here. Therefore, ``vo.table`` disallows the colon.
colon here. Therefore, ``astropy.io.votable`` disallows the colon.

VOTable 1.2 corrects this error in the specification.

Expand Down Expand Up @@ -323,7 +331,7 @@ class W03(VOTableChangeWarning):
``name`` attributes of ``FIELD``, ``PARAM`` and optional
``GROUP`` elements should be all different.

Since ``vo.table`` requires a unique identifier for each of its
Since ``astropy.io.votable`` requires a unique identifier for each of its
columns, ``ID`` is used for the column name when present.
However, when ``ID`` is not present, (since it is not required by
the specification) ``name`` is used instead. However, ``name``
Expand Down Expand Up @@ -415,7 +423,7 @@ class W07(VOTableSpecWarning):

class W08(VOTableSpecWarning):
"""
To avoid local-dependent number parsing differences, ``vo.table``
To avoid local-dependent number parsing differences, ``astropy.io.votable``
may require a string or unicode string where a numeric type may
make more sense.
"""
Expand All @@ -430,8 +438,8 @@ class W09(VOTableSpecWarning):
The VOTable specification uses the attribute name ``ID`` (with
uppercase letters) to specify unique identifiers. Some
VOTable-producing tools use the more standard lowercase ``id``
instead. ``vo.table`` accepts ``id`` and emits this warning when
not in ``pedantic`` mode.
instead. ``astropy.io.votable`` accepts ``id`` and emits this warning if
``verify`` is ``'warn'``.

**References**: `1.1
<http://www.ivoa.net/Documents/VOTable/20040811/REC-VOTable-1.1-20040811.html#sec:name>`__,
Expand All @@ -449,7 +457,7 @@ class W10(VOTableSpecWarning):
against the VOTable schema (with a tool such as `xmllint
<http://xmlsoft.org/xmllint.html>`__. If the file validates
against the schema, and you still receive this warning, this may
indicate a bug in ``vo.table``.
indicate a bug in ``astropy.io.votable``.

**References**: `1.1
<http://www.ivoa.net/Documents/VOTable/20040811/REC-VOTable-1.1-20040811.html#ToC54>`__,
Expand All @@ -468,7 +476,7 @@ class W11(VOTableSpecWarning):
<http://aladin.u-strasbg.fr/glu/>`__. New files should
specify a ``glu:`` protocol using the ``href`` attribute.

Since ``vo.table`` does not currently support GLU references, it
Since ``astropy.io.votable`` does not currently support GLU references, it
likewise does not automatically convert the ``gref`` attribute to
the new form.

Expand All @@ -487,8 +495,8 @@ class W12(VOTableChangeWarning):
``FIELD`` element must have either an ``ID`` or ``name`` attribute
to derive a name from. Strictly speaking, according to the
VOTable schema, the ``name`` attribute is required. However, if
``name`` is not present by ``ID`` is, and *pedantic mode* is off,
``vo.table`` will continue without a ``name`` defined.
``name`` is not present by ``ID`` is, and ``verify`` is not ``'exception'``,
``astropy.io.votable`` will continue without a ``name`` defined.

**References**: `1.1
<http://www.ivoa.net/Documents/VOTable/20040811/REC-VOTable-1.1-20040811.html#sec:name>`__,
Expand Down Expand Up @@ -536,8 +544,8 @@ class W15(VOTableSpecWarning):
"""
The ``name`` attribute is required on every ``FIELD`` element.
However, many VOTable files in the wild omit it and provide only
an ``ID`` instead. In this case, when *pedantic mode* is off,
``vo.table`` will copy the ``name`` attribute to a new ``ID``
an ``ID`` instead. In this case, when ``verify`` is not ``'exception'``
``astropy.io.votable`` will copy the ``name`` attribute to a new ``ID``
attribute.

**References**: `1.1
Expand Down Expand Up @@ -576,8 +584,8 @@ class W18(VOTableSpecWarning):
The number of rows explicitly specified in the ``nrows`` attribute
does not match the actual number of rows (``TR`` elements) present
in the ``TABLE``. This may indicate truncation of the file, or an
internal error in the tool that produced it. If *pedantic mode*
is off, parsing will proceed, with the loss of some performance.
internal error in the tool that produced it. If ``verify`` is not
``'exception'``, parsing will proceed, with the loss of some performance.

**References:** `1.1
<http://www.ivoa.net/Documents/VOTable/20040811/REC-VOTable-1.1-20040811.html#ToC10>`__,
Expand All @@ -592,8 +600,8 @@ class W18(VOTableSpecWarning):
class W19(VOTableSpecWarning):
"""
The column fields as defined using ``FIELD`` elements do not match
those in the headers of the embedded FITS file. If *pedantic
mode* is off, the embedded FITS file will take precedence.
those in the headers of the embedded FITS file. If ``verify`` is not
``'exception'``, the embedded FITS file will take precedence.
"""

message_template = (
Expand All @@ -613,12 +621,12 @@ class W20(VOTableSpecWarning):

class W21(UnimplementedWarning):
"""
Unknown issues may arise using ``vo.table`` with VOTable files
Unknown issues may arise using ``astropy.io.votable`` with VOTable files
from a version other than 1.1, 1.2 or 1.3.
"""

message_template = (
'vo.table is designed for VOTable version 1.1, 1.2 and 1.3, but ' +
'astropy.io.votable is designed for VOTable version 1.1, 1.2 and 1.3, but ' +
'this file is {}')
default_args = ('x',)

Expand Down Expand Up @@ -653,12 +661,12 @@ class W23(IOWarning):
class W24(VOWarning, FutureWarning):
"""
The VO catalog database retrieved from the www is designed for a
newer version of vo.table. This may cause problems or limited
features performing service queries. Consider upgrading vo.table
newer version of ``astropy.io.votable``. This may cause problems or limited
features performing service queries. Consider upgrading ``astropy.io.votable``
to the latest version.
"""

message_template = "The VO catalog database is for a later version of vo.table"
message_template = "The VO catalog database is for a later version of astropy.io.votable"


class W25(IOWarning):
Expand Down Expand Up @@ -726,9 +734,9 @@ class W29(VOTableSpecWarning):

class W30(VOTableSpecWarning):
"""
Some VOTable files write missing floating-point values in non-standard
ways, such as "null" and "-". In non-pedantic mode, any non-standard
floating-point literals are treated as missing values.
Some VOTable files write missing floating-point values in non-standard ways,
such as "null" and "-". If ``verify`` is not ``'exception'``, any
non-standard floating-point literals are treated as missing values.

**References**: `1.1
<http://www.ivoa.net/Documents/VOTable/20040811/REC-VOTable-1.1-20040811.html#sec:datatypes>`__,
Expand Down Expand Up @@ -840,7 +848,7 @@ class W36(VOTableSpecWarning):
class W37(UnimplementedWarning):
"""
The 3 datatypes defined in the VOTable specification and supported by
vo.table are ``TABLEDATA``, ``BINARY`` and ``FITS``.
``astropy.io.votable`` are ``TABLEDATA``, ``BINARY`` and ``FITS``.

**References:** `1.1
<http://www.ivoa.net/Documents/VOTable/20040811/REC-VOTable-1.1-20040811.html#sec:data>`__,
Expand Down