From feb0f6ce44ba0e31e9d534eac08aa435ea6a55db Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Tue, 29 Nov 2016 17:49:30 +0100 Subject: [PATCH 1/3] support more options like keep and mapping to wrapped function names --- CHANGES.rst | 2 + astropy/nddata/decorators.py | 326 ++++++++++++++++-------- astropy/nddata/tests/test_decorators.py | 148 +++++++++++ 3 files changed, 364 insertions(+), 112 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index b5e463dc3b05..696de54a9415 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -76,6 +76,8 @@ New Features - ``astropy.nddata`` + - Added ``keep`` and ``**kwargs`` parameter to ``support_nddata``. [#5477] + - ``astropy.stats`` - Added ``axis`` keyword to ``biweight_location`` and diff --git a/astropy/nddata/decorators.py b/astropy/nddata/decorators.py index 64dd26e8a085..07ac590c9c27 100644 --- a/astropy/nddata/decorators.py +++ b/astropy/nddata/decorators.py @@ -3,11 +3,16 @@ from __future__ import (absolute_import, division, print_function, unicode_literals) +from copy import deepcopy +from itertools import islice import warnings +import numpy as np + from ..utils import wraps from ..utils.exceptions import AstropyUserWarning from ..utils.compat.funcsigs import signature +from ..extern import six from ..extern.six.moves import zip from .nddata import NDData @@ -15,149 +20,246 @@ __all__ = ['support_nddata'] -def support_nddata(_func=None, accepts=NDData, repack=False, returns=None): - """ - Decorator to split NDData properties into function arguments. - - This is a decorator to allow functions to take NDData objects as their - first arguments and split up the properties into kwargs as required by the - function. For example, if you consider the following function:: - - def downsample(data, wcs=None): - # downsample data and optionally WCS here - pass - - This function takes a Numpy array for the data, and some WCS information - with the ``data`` keyword argument. However, you might have an NDData - instance that has the ``wcs`` property set and you would like to be able to - call the function with ``downsample(my_nddata)`` and have the WCS - information, if present, automatically be passed to the ``wcs`` keyword - argument. - - This decorator can be used to make this possible:: - - @support_nddata - def downsample(data, wcs=None): - # downsample data and optionally WCS here - pass - - This function can now either be called as before, specifying the data and - WCS separately, or an NDData instance can be passed to the ``data`` - argument. - - The restrictions on functions to use this function are: - - * The first positional argument should be ``data`` and take a Numpy array. - - * The following arguments can optionally be specified in the function - signature, but if they are specified they should be keyword arguments: - ``uncertainty``, ``mask``, ``meta``, ``unit``, and ``wcs``. If - you are making use of this decorator, you should be prepared for these - keyword arguments to be set to the properties of the NDData object (if - present). - - The behavior of the decorator is to check through the NDData properties and - if they are set, it checks if the function accepts them as keyword - arguments. If an NDData property is set but cannot be passed to a keyword - argument, a warning is emitted to tell the user that the NDData property in - question will not be used by the function (to ensure that they know when - e.g. uncertainties cannot be used). - - If the user passes an NDData object *and* explicitly sets a keyword - argument that is one of the valid NDData properties, a warning is emitted - to inform the user that the explicitly specified value will take priority. +# All supported properties except "data" which is mandatory! +SUPPORTED_PROPERTIES = ['data', 'uncertainty', 'mask', 'meta', 'unit', 'wcs', + 'flags'] + + +def support_nddata(_func=None, accepts=NDData, + repack=False, returns=None, keeps=None, + **attribute_argument_mapping): + """Decorator to wrap functions that could accept an unwrapped NDData + instance. + + Parameters + ---------- + _func : callable, None, optional + The function to decorate or ``None`` if used as factory. + Default is ``None``. + + accepts : cls, optional + The class or subclass of ``NDData`` that should be unpacked before + calling the function. + Default is ``NDData`` + + repack : bool, optional + Should be ``True`` if the return should be converted to the input + class again after the wrapped function call. + Default is ``False``. + + .. note:: + Must be ``True`` if either one of ``returns`` or ``keeps`` + is specified. + + returns : iterable, None, optional + An iterable containing strings which returned value should be set + on the class. For example if a function returns data and mask, this + should be ``['data', 'mask']``. If ``None`` assume the function only + returns one argument: ``'data'``. + Default is ``None``. + + .. note:: + Must be ``None`` if ``repack=False``. + + keeps : iterable. None, optional + An iterable containing strings that indicate which values should be + copied from the original input to the returned class. If ``None`` + assume that no attributes are copied. + Default is ``None``. + + .. note:: + Must be ``None`` if ``repack=False``. + + .. warning:: + If the decorated function should work with ``ccdproc.CCDData`` or + similar, you *probably* need to specify ``keeps=['unit']``, unless + the function returns a `~astropy.units.Quantity` or CCDData-like + object with unit. + + attribute_argument_mapping : + Keyword parameters that optionally indicate which function argument + should be interpreted as which attribute on the input. By default + it assumes the function takes a ``data`` argument as first argument, + but if the first argument is called ``input`` one should pass + ``support_nddata(..., data='input')`` to the function. + + Returns + ------- + decorator_factory or decorated_function : callable + If ``_func=None`` this returns a decorator, otherwise it returns the + decorated ``_func``. + + Notes + ----- + This is a slightly modified version of `~astropy.nddata.support_nddata`, so + for more hints checkout their documentation or have a look at the + ``ccdproc.core.py`` code. """ - - if returns is not None and not repack: - raise ValueError('returns should only be set if repack=True') - - if returns is None and repack: - raise ValueError('returns should be set if repack=True') + if (returns is not None or keeps is not None) and not repack: + raise ValueError('returns or keep should only be set if repack=True.') + elif returns is None and repack: + raise ValueError('returns should be set if repack=True.') + else: + # Use empty lists for returns and keeps so we don't need to check + # if any of those is None later on. + if returns is None: + returns = [] + if keeps is None: + keeps = [] + + # Short version to avoid the long variable name later. + attr_arg_map = attribute_argument_mapping + if any(keep in returns for keep in keeps): + raise ValueError("cannot specify the same attribute in `returns` and " + "`keeps`.") + all_returns = returns + keeps def support_nddata_decorator(func): - # Find out args and kwargs - sig = signature(func) - func_args = [] - func_kwargs = [] - for param in sig.parameters.values(): + func_args, func_kwargs = [], [] + sig = signature(func).parameters + for param_name, param in six.iteritems(sig): if param.kind in (param.VAR_POSITIONAL, param.VAR_KEYWORD): - raise ValueError("func may not have *args or **kwargs") + raise ValueError("func may not have *args or **kwargs.") elif param.default == param.empty: - func_args.append(param.name) + func_args.append(param_name) else: - func_kwargs.append(param.name) + func_kwargs.append(param_name) # First argument should be data - if len(func_args) == 0 or func_args[0] != 'data': - raise ValueError("Can only wrap functions whose first positional argument is `data`") - - supported_properties = ['uncertainty', 'mask', 'meta', 'unit', 'wcs'] + if not func_args or func_args[0] != attr_arg_map.get('data', 'data'): + raise ValueError("Can only wrap functions whose first positional " + "argument is `{0}`" + "".format(attr_arg_map.get('data', 'data'))) + + # Get all supported properties that match a parameter in the signature. + supported_properties = [prop for prop in islice(SUPPORTED_PROPERTIES, 1, None) + if attr_arg_map.get(prop, prop) in func_kwargs] + + """ + # Create a Table to store the information about the wrapped function. + # Can be used to create a Table that can be inserted in the docstring. + # Note: Creating and writing an astropy Table takes very long so + # creating the docstring at import time may be a severe time + # consumer... + # Maybe worth investigating if some templating engine might + # generate them faster. + from astropy.io.ascii import RST + from astropy.table import Table + + _names = SUPPORTED_PROPERTIES + _used, _calc, _copy = [], [], [] + for prop in _names: + _used.append('X' if prop in supported_properties else '--') + _calc.append('X' if prop in returns else '--') + _copy.append('X' if prop in keeps else '--') + # Data is always used! + _used[0] = 'X' + _tbl = Table([_names, _used, _calc, _copy], + names=('attribute', 'used', 'calculated', 'copied')) + _tbl = ascii.RST().write(_tbl) + _doc = '\n'.join(_tbl) + print(_doc) # print to get the nice output. + # # End of Table creation. + """ @wraps(func) def wrapper(data, *args, **kwargs): - unpack = isinstance(data, accepts) input_data = data - + ignored = [] if not unpack and isinstance(data, NDData): raise TypeError("Only NDData sub-classes that inherit from {0}" - " can be used by this function".format(accepts.__name__)) + " can be used by this function" + "".format(accepts.__name__)) - # If data is an NDData instance, we can try and find properties that - # can be passed as kwargs. + # If data is an NDData instance, we can try and find properties + # that can be passed as kwargs. if unpack: - - ignored = [] - # We loop over a list of pre-defined properties - for prop in supported_properties: - - # We only need to do something if the property exists on the - # NDData object - if hasattr(data, prop): + for prop in islice(SUPPORTED_PROPERTIES, 1, None): + # We only need to do something if the property exists on + # the NDData object + try: value = getattr(data, prop) - if (prop == 'meta' and len(value) > 0) or (prop != 'meta' and value is not None): - if prop in func_kwargs: - if prop in kwargs and kwargs[prop] is not None: - warnings.warn("Property {0} has been passed explicitly and as an " - "NDData property, using explicitly specified value".format(prop), - AstropyUserWarning) - else: - kwargs[prop] = value - else: - ignored.append(prop) + except AttributeError: + continue + # Skip if the property exists but is None or empty. + if prop == 'meta' and not value: + continue + elif value is None: + continue + # Warn if the property is set and explicitly given + propmatch = attr_arg_map.get(prop, prop) + + if propmatch not in func_kwargs: + ignored.append(prop) + continue + + # Check if the property was explicitly given and issue a + # Warning if it is. + if propmatch in kwargs: + # If it's in the func_args it's trivial but if it was + # in the func_kwargs we need to compare it to the + # default. + # FIXME: This obviously fails if the explicitly given + # value is identical to the default. No idea how that + # could be achieved without replacing the signature or + # parts of it. That would have other drawbacks like + # making double-decorating nearly impossible and + # violating the sphinx-documentation for that function. + if (propmatch in func_args or + (propmatch in func_kwargs and + not np.array_equal(kwargs[propmatch], + sig[propmatch].default))): + warnings.warn( + "Property {0} has been passed explicitly and " + "as an NDData property{1}, using explicitly " + "specified value" + "".format(propmatch, '' if prop == propmatch + else ' ' + prop), + AstropyUserWarning) + continue + # Otherwise use the property as input for the function. + kwargs[propmatch] = value + # Finally, replace data by the data itself + data = data.data if ignored: - warnings.warn("The following attributes were set on the data object, " - "but will be ignored by the function: " + ", ".join(ignored), + warnings.warn("The following attributes were set on the " + "data object, but will be ignored by the " + "function: " + ", ".join(ignored), AstropyUserWarning) - # Finally, replace data by the data itself - data = data.data - result = func(data, *args, **kwargs) - if unpack: - - if repack: - if len(returns) > 1 and len(returns) != len(result): - raise ValueError("Function did not return the expected number of arguments") - elif len(returns) == 1: - result = [result] - return input_data.__class__(**dict(zip(returns, result))) - else: - return result - + if unpack and repack: + # If there are multiple required returned arguments make sure + # the result is a tuple (because we don't want to unpack + # numpy arrays or compare to their length, never!) and has the + # same length. + if len(returns) > 1: + if (not isinstance(result, tuple) or + len(returns) != len(result)): + raise ValueError("Function did not return the " + "expected number of arguments.") + elif len(returns) == 1: + result = [result] + if keeps is not None: + for keep in keeps: + result.append(deepcopy(getattr(input_data, keep))) + resultdata = result[all_returns.index('data')] + resultkwargs = {ret: res + for ret, res in zip(all_returns, result) + if ret != 'data'} + return input_data.__class__(resultdata, **resultkwargs) else: - return result - return wrapper # If _func is set, this means that the decorator was used without - # parameters so we have to return the result of the support_nddata_decorator - # decorator rather than the decorator itself + # parameters so we have to return the result of the + # support_nddata_decorator decorator rather than the decorator itself if _func is not None: return support_nddata_decorator(_func) else: diff --git a/astropy/nddata/tests/test_decorators.py b/astropy/nddata/tests/test_decorators.py index 54c4520ce0fe..394a6cba418e 100644 --- a/astropy/nddata/tests/test_decorators.py +++ b/astropy/nddata/tests/test_decorators.py @@ -15,6 +15,10 @@ from ..decorators import support_nddata +class CCDData(NDData): + pass + + @support_nddata def wrapped_function_1(data, wcs=None, unit=None): return data, wcs, unit @@ -187,3 +191,147 @@ def wrapped_function_6(data, wcs=None, unit=None): *inspect.getfullargspec(wrapped_function_6)) assert signature == "(data, wcs=None, unit=None)" + + +def test_setup_failures1(): + # repack but no returns + with pytest.raises(ValueError): + support_nddata(repack=True) + + +def test_setup_failures2(): + # returns but no repack + with pytest.raises(ValueError): + support_nddata(returns=['data']) + + +def test_setup_failures9(): + # keeps but no repack + with pytest.raises(ValueError): + support_nddata(keeps=['unit']) + + +def test_setup_failures3(): + # same attribute in keeps and returns + with pytest.raises(ValueError): + support_nddata(repack=True, keeps=['mask'], returns=['data', 'mask']) + + +def test_setup_failures4(): + # function accepts *args + with pytest.raises(ValueError): + @support_nddata + def test(data, *args): + pass + + +def test_setup_failures10(): + # function accepts **kwargs + with pytest.raises(ValueError): + @support_nddata + def test(data, **kwargs): + pass + + +def test_setup_failures5(): + # function accepts *args (or **kwargs) + with pytest.raises(ValueError): + @support_nddata + def test(data, *args): + pass + + +def test_setup_failures6(): + # First argument is not data + with pytest.raises(ValueError): + @support_nddata + def test(img): + pass + + +def test_setup_failures7(): + # accepts CCDData but was given just an NDData + with pytest.raises(TypeError): + @support_nddata(accepts=CCDData) + def test(data): + pass + test(NDData(np.ones((3, 3)))) + + +def test_setup_failures8(): + # function returns a different amount of arguments than specified. Using + # NDData here so we don't get into troubles when creating a CCDData without + # unit! + with pytest.raises(ValueError): + @support_nddata(repack=True, returns=['data', 'mask']) + def test(data): + return 10 + test(NDData(np.ones((3, 3)))) # do NOT use CCDData here. + + +def test_setup_failures11(): + # function accepts no arguments + with pytest.raises(ValueError): + @support_nddata + def test(): + pass + + +def test_still_accepts_other_input(): + @support_nddata(repack=True, returns=['data']) + def test(data): + return data + assert isinstance(test(NDData(np.ones((3, 3)))), NDData) + assert isinstance(test(10), int) + assert isinstance(test([1, 2, 3]), list) + + +def test_accepting_property_normal(): + # Accepts a mask attribute and takes it from the input + @support_nddata + def test(data, mask=None): + return mask + + ndd = NDData(np.ones((3, 3))) + assert test(ndd) is None + ndd._mask = np.zeros((3, 3)) + assert np.all(test(ndd) == 0) + # Use the explicitly given one (raises a Warning but too lazy to check!) + assert test(ndd, mask=10) == 10 + + +def test_accepting_property_notexist(): + # Accepts flags attribute but NDData doesn't have one + @support_nddata + def test(data, flags=10): + return flags + + ndd = NDData(np.ones((3, 3))) + test(ndd) + + +def test_accepting_property_translated(): + # Accepts a error attribute and we want to pass in uncertainty! + @support_nddata(mask='masked') + def test(data, masked=None): + return masked + + ndd = NDData(np.ones((3, 3))) + assert test(ndd) is None + ndd._mask = np.zeros((3, 3)) + assert np.all(test(ndd) == 0) + # Use the explicitly given one (raises a Warning but too lazy to check!) + assert test(ndd, masked=10) == 10 + + +def test_accepting_property_meta_empty(): + # Meta is always set (OrderedDict) so it has a special case that it's + # ignored if it's empty but not None + @support_nddata + def test(data, meta=None): + return meta + + ndd = NDData(np.ones((3, 3))) + assert test(ndd) is None + ndd._meta = {'a': 10} + assert test(ndd) == {'a': 10} From f9df883cadda6f1ad4d322b5ec909081db8c3aef Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Tue, 29 Nov 2016 18:49:26 +0100 Subject: [PATCH 2/3] updated narrative docs and included feedback from PR comments --- astropy/nddata/decorators.py | 67 +++++++++++------------------------- docs/nddata/decorator.rst | 7 ++-- 2 files changed, 23 insertions(+), 51 deletions(-) diff --git a/astropy/nddata/decorators.py b/astropy/nddata/decorators.py index 07ac590c9c27..2760e9709607 100644 --- a/astropy/nddata/decorators.py +++ b/astropy/nddata/decorators.py @@ -28,8 +28,8 @@ def support_nddata(_func=None, accepts=NDData, repack=False, returns=None, keeps=None, **attribute_argument_mapping): - """Decorator to wrap functions that could accept an unwrapped NDData - instance. + """Decorator to wrap functions that could accept an NDData instance with + its properties passed as function arguments. Parameters ---------- @@ -70,12 +70,6 @@ class again after the wrapped function call. .. note:: Must be ``None`` if ``repack=False``. - .. warning:: - If the decorated function should work with ``ccdproc.CCDData`` or - similar, you *probably* need to specify ``keeps=['unit']``, unless - the function returns a `~astropy.units.Quantity` or CCDData-like - object with unit. - attribute_argument_mapping : Keyword parameters that optionally indicate which function argument should be interpreted as which attribute on the input. By default @@ -91,12 +85,23 @@ class again after the wrapped function call. Notes ----- - This is a slightly modified version of `~astropy.nddata.support_nddata`, so - for more hints checkout their documentation or have a look at the - ``ccdproc.core.py`` code. + If properties of ``NDData`` are set but have no corresponding function + argument a Warning is shown. + + If a property is set of the ``NDData`` are set and an explicit argument is + given, the explicitly given argument is used and a Warning is shown. + + The supported properties are: + + - ``mask`` + - ``unit`` + - ``wcs`` + - ``meta`` + - ``uncertainty`` + - ``flags`` """ if (returns is not None or keeps is not None) and not repack: - raise ValueError('returns or keep should only be set if repack=True.') + raise ValueError('returns or keeps should only be set if repack=True.') elif returns is None and repack: raise ValueError('returns should be set if repack=True.') else: @@ -132,37 +137,6 @@ def support_nddata_decorator(func): "argument is `{0}`" "".format(attr_arg_map.get('data', 'data'))) - # Get all supported properties that match a parameter in the signature. - supported_properties = [prop for prop in islice(SUPPORTED_PROPERTIES, 1, None) - if attr_arg_map.get(prop, prop) in func_kwargs] - - """ - # Create a Table to store the information about the wrapped function. - # Can be used to create a Table that can be inserted in the docstring. - # Note: Creating and writing an astropy Table takes very long so - # creating the docstring at import time may be a severe time - # consumer... - # Maybe worth investigating if some templating engine might - # generate them faster. - from astropy.io.ascii import RST - from astropy.table import Table - - _names = SUPPORTED_PROPERTIES - _used, _calc, _copy = [], [], [] - for prop in _names: - _used.append('X' if prop in supported_properties else '--') - _calc.append('X' if prop in returns else '--') - _copy.append('X' if prop in keeps else '--') - # Data is always used! - _used[0] = 'X' - _tbl = Table([_names, _used, _calc, _copy], - names=('attribute', 'used', 'calculated', 'copied')) - _tbl = ascii.RST().write(_tbl) - _doc = '\n'.join(_tbl) - print(_doc) # print to get the nice output. - # # End of Table creation. - """ - @wraps(func) def wrapper(data, *args, **kwargs): unpack = isinstance(data, accepts) @@ -189,9 +163,8 @@ def wrapper(data, *args, **kwargs): continue elif value is None: continue - # Warn if the property is set and explicitly given + # Warn if the property is set but not used by the function. propmatch = attr_arg_map.get(prop, prop) - if propmatch not in func_kwargs: ignored.append(prop) continue @@ -222,7 +195,7 @@ def wrapper(data, *args, **kwargs): continue # Otherwise use the property as input for the function. kwargs[propmatch] = value - # Finally, replace data by the data itself + # Finally, replace data by the data attribute data = data.data if ignored: @@ -236,7 +209,7 @@ def wrapper(data, *args, **kwargs): if unpack and repack: # If there are multiple required returned arguments make sure # the result is a tuple (because we don't want to unpack - # numpy arrays or compare to their length, never!) and has the + # numpy arrays or compare their length, never!) and has the # same length. if len(returns) > 1: if (not isinstance(result, tuple) or diff --git a/docs/nddata/decorator.rst b/docs/nddata/decorator.rst index c79c1a4dc9ac..adb09a433529 100644 --- a/docs/nddata/decorator.rst +++ b/docs/nddata/decorator.rst @@ -41,11 +41,10 @@ of the arguments are also properties of the ``NDData`` object, and passes them as individual arguments. The function can also be called with separate arguments as if it wasn't decorated. -An exception is raised if an ``NDData`` property is set but the function does +An warning is emitted if an ``NDData`` property is set but the function does not accept it - for example, if ``wcs`` is set, but the function cannot support -WCS objects, an error would be raised. On the other hand, if an argument in the -function does not exist in the ``NDData`` object or is not set, it is simply -left to its default value. +WCS objects. On the other hand, if an argument in the function does not exist +in the ``NDData`` object or is not set, it is simply left to its default value. If the function call succeeds, then the decorator returns the values from the function unmodified by default. However, in some cases we may want to return From 11535dadb0153641145d764ff43c47cd43580d2f Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Thu, 1 Dec 2016 22:42:48 +0100 Subject: [PATCH 3/3] allow numpy-arrays as parameter default value. Also uses explicitly passed in argument if it is not identical (same id) to the default. Before it compared equality instead of identity. Includes small documentation fixes suggested by @mwcraig. --- astropy/nddata/decorators.py | 48 ++++++++++++++++--------- astropy/nddata/tests/test_decorators.py | 37 ++++++++++++++++--- 2 files changed, 65 insertions(+), 20 deletions(-) diff --git a/astropy/nddata/decorators.py b/astropy/nddata/decorators.py index 2760e9709607..a0beb29d9b25 100644 --- a/astropy/nddata/decorators.py +++ b/astropy/nddata/decorators.py @@ -7,8 +7,6 @@ from itertools import islice import warnings -import numpy as np - from ..utils import wraps from ..utils.exceptions import AstropyUserWarning from ..utils.compat.funcsigs import signature @@ -20,7 +18,7 @@ __all__ = ['support_nddata'] -# All supported properties except "data" which is mandatory! +# All supported properties are optional except "data" which is mandatory! SUPPORTED_PROPERTIES = ['data', 'uncertainty', 'mask', 'meta', 'unit', 'wcs', 'flags'] @@ -34,7 +32,10 @@ def support_nddata(_func=None, accepts=NDData, Parameters ---------- _func : callable, None, optional - The function to decorate or ``None`` if used as factory. + The function to decorate or ``None`` if used as factory. The first + positional argument should be ``data`` and take a numpy array. It is + possible to overwrite the name, see ``attribute_argument_mapping`` + argument. Default is ``None``. accepts : cls, optional @@ -126,10 +127,21 @@ def support_nddata_decorator(func): for param_name, param in six.iteritems(sig): if param.kind in (param.VAR_POSITIONAL, param.VAR_KEYWORD): raise ValueError("func may not have *args or **kwargs.") - elif param.default == param.empty: - func_args.append(param_name) - else: - func_kwargs.append(param_name) + try: + if param.default == param.empty: + func_args.append(param_name) + else: + func_kwargs.append(param_name) + # The comparison to param.empty may fail if the default is a + # numpy array or something similar. So if the comparison fails then + # it's quite obvious that there was a default and it should be + # appended to the "func_kwargs". + except ValueError as exc: + if ('The truth value of an array with more than one element ' + 'is ambiguous.') in str(exc): + func_kwargs.append(param_name) + else: + raise # First argument should be data if not func_args or func_args[0] != attr_arg_map.get('data', 'data'): @@ -175,16 +187,20 @@ def wrapper(data, *args, **kwargs): # If it's in the func_args it's trivial but if it was # in the func_kwargs we need to compare it to the # default. - # FIXME: This obviously fails if the explicitly given - # value is identical to the default. No idea how that - # could be achieved without replacing the signature or - # parts of it. That would have other drawbacks like - # making double-decorating nearly impossible and - # violating the sphinx-documentation for that function. + # Comparison to the default is done by comparing their + # identity, this works because defaults in function + # signatures are only created once and always reference + # the same item. + # FIXME: Python interns some values, for example the + # integers from -5 to 255 (any maybe some other types + # as well). In that case the default is + # indistinguishable from an explicitly passed kwarg + # and it won't notice that and use the attribute of the + # NDData. if (propmatch in func_args or (propmatch in func_kwargs and - not np.array_equal(kwargs[propmatch], - sig[propmatch].default))): + (kwargs[propmatch] is not + sig[propmatch].default))): warnings.warn( "Property {0} has been passed explicitly and " "as an NDData property{1}, using explicitly " diff --git a/astropy/nddata/tests/test_decorators.py b/astropy/nddata/tests/test_decorators.py index 394a6cba418e..8b79bdb688cd 100644 --- a/astropy/nddata/tests/test_decorators.py +++ b/astropy/nddata/tests/test_decorators.py @@ -9,6 +9,7 @@ from ...extern import six from ...tests.helper import catch_warnings, pytest +from ...utils.exceptions import AstropyUserWarning from ... import units as u from ..nddata import NDData @@ -277,6 +278,14 @@ def test(): pass +def test_setup_numpyarray_default(): + # It should be possible (even if it's not advisable to use mutable + # defaults) to have a numpy array as default value. + @support_nddata + def func(data, wcs=np.array([1, 2, 3])): + return wcs + + def test_still_accepts_other_input(): @support_nddata(repack=True, returns=['data']) def test(data): @@ -296,8 +305,26 @@ def test(data, mask=None): assert test(ndd) is None ndd._mask = np.zeros((3, 3)) assert np.all(test(ndd) == 0) - # Use the explicitly given one (raises a Warning but too lazy to check!) - assert test(ndd, mask=10) == 10 + # Use the explicitly given one (raises a Warning) + with catch_warnings(AstropyUserWarning) as w: + assert test(ndd, mask=10) == 10 + assert len(w) == 1 + + +def test_parameter_default_identical_to_explicit_passed_argument(): + # If the default is identical to the explicitly passed argument this + # should still raise a Warning and use the explicit one. + @support_nddata + def func(data, wcs=[1, 2, 3]): + return wcs + + with catch_warnings(AstropyUserWarning) as w: + assert func(NDData(1, wcs=[1, 2]), [1, 2, 3]) == [1, 2, 3] + assert len(w) == 1 + + with catch_warnings(AstropyUserWarning) as w: + assert func(NDData(1, wcs=[1, 2])) == [1, 2] + assert len(w) == 0 def test_accepting_property_notexist(): @@ -320,8 +347,10 @@ def test(data, masked=None): assert test(ndd) is None ndd._mask = np.zeros((3, 3)) assert np.all(test(ndd) == 0) - # Use the explicitly given one (raises a Warning but too lazy to check!) - assert test(ndd, masked=10) == 10 + # Use the explicitly given one (raises a Warning) + with catch_warnings(AstropyUserWarning) as w: + assert test(ndd, masked=10) == 10 + assert len(w) == 1 def test_accepting_property_meta_empty():