Skip to content

Commit

Permalink
allow numpy-arrays as parameter default value.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
MSeifert04 committed Dec 1, 2016
1 parent f9df883 commit 11535da
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 20 deletions.
48 changes: 32 additions & 16 deletions astropy/nddata/decorators.py
Expand Up @@ -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
Expand All @@ -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']

Expand All @@ -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
Expand Down Expand Up @@ -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'):
Expand Down Expand Up @@ -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 "
Expand Down
37 changes: 33 additions & 4 deletions astropy/nddata/tests/test_decorators.py
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -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():
Expand All @@ -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():
Expand Down

0 comments on commit 11535da

Please sign in to comment.