From d65d668e3d64c5726705b05a9b4c2bc45b5e1c53 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Thu, 29 Jul 2021 15:42:19 -0400 Subject: [PATCH] Removed the depreciated `ExpressionTree` class and associated tests. Removed deprecated models `MexicanHat1D` and `MexicanHat2D`. Removed deprecated code in `~astropy/modeling/core.py`. Removed deprecation filtering from tests. Fixed codestyle error. Added changelog entry. Updated changelog entry. --- astropy/modeling/core.py | 52 +--- astropy/modeling/functional_models.py | 11 - astropy/modeling/models.py | 3 - .../modeling/tests/test_functional_models.py | 15 +- .../modeling/tests/test_quantities_model.py | 6 +- astropy/modeling/tests/test_utils.py | 100 ------- astropy/modeling/utils.py | 283 +----------------- docs/changes/modeling/11978.api.rst | 7 + 8 files changed, 14 insertions(+), 463 deletions(-) create mode 100644 docs/changes/modeling/11978.api.rst diff --git a/astropy/modeling/core.py b/astropy/modeling/core.py index 834284233c3..5466f0d1f3d 100644 --- a/astropy/modeling/core.py +++ b/astropy/modeling/core.py @@ -20,7 +20,6 @@ import functools import operator import types -import warnings from collections import defaultdict, deque from inspect import signature @@ -35,7 +34,6 @@ from astropy.utils import (sharedmethod, find_current_module, check_broadcast, IncompatibleShapeError, isiterable) from astropy.utils.codegen import make_function_with_signature -from astropy.utils.exceptions import AstropyDeprecationWarning from astropy.nddata.utils import add_array, extract_array from .utils import (combine_labels, make_binary_operator_eval, get_inputs_and_params, _BoundingBox, _combine_equivalency_dict, @@ -719,19 +717,6 @@ def __init__(self, *args, meta=None, name=None, **kwargs): self._initialize_slices() self._initialize_unit_support() - # Raise DeprecationWarning on classes with class attributes - # ``inputs`` and ``outputs``. - self._inputs_deprecation() - - def _inputs_deprecation(self): - if hasattr(self.__class__, 'inputs') and isinstance(self.__class__.inputs, tuple): - warnings.warn( - f"""Class {self.__class__.__name__} defines class attributes ``inputs``. - This has been deprecated in v4.0 and support will be removed in v4.1. - Starting with v4.0 classes must define a class attribute ``n_inputs``. - Please consult the documentation for details. - """, AstropyDeprecationWarning) - def _default_inputs_outputs(self): if self.n_inputs == 1 and self.n_outputs == 1: self._inputs = ("x",) @@ -2519,7 +2504,7 @@ class CompoundModel(Model): to combine models is through the model operators. ''' - def __init__(self, op, left, right, name=None, inverse=None): + def __init__(self, op, left, right, name=None): self.__dict__['_param_names'] = None self._n_submodels = None self.op = op @@ -2533,14 +2518,6 @@ def __init__(self, op, left, right, name=None, inverse=None): self._parameters_ = None self._param_metrics = None - if inverse: - warnings.warn( - "The 'inverse' argument is deprecated. Instead, set the inverse " - "property after CompoundModel is initialized.", - AstropyDeprecationWarning - ) - self.inverse = inverse - if op != 'fix_inputs' and len(left) != len(right): raise ValueError( 'Both operands must have equal values for n_models') @@ -2785,24 +2762,6 @@ def submodel_names(self): newnames.append(item) return tuple(newnames) - def both_inverses_exist(self): - ''' - if both members of this compound model have inverses return True - ''' - warnings.warn( - "CompoundModel.both_inverses_exist is deprecated. " - "Use has_inverse instead.", - AstropyDeprecationWarning - ) - - try: - linv = self.left.inverse - rinv = self.right.inverse - except NotImplementedError: - return False - - return True - def __call__(self, *args, **kw): # Turn any keyword arguments into positional arguments. args, kw = self._get_renamed_inputs_as_positional(*args, **kw) @@ -3618,7 +3577,7 @@ def fix_inputs(modelinstance, values): return CompoundModel('fix_inputs', modelinstance, values) -def custom_model(*args, fit_deriv=None, **kwargs): +def custom_model(*args, fit_deriv=None): """ Create a model from a user defined function. The inputs and parameters of the model will be inferred from the arguments of the function. @@ -3689,13 +3648,6 @@ def custom_model(*args, fit_deriv=None, **kwargs): 0.3333333333333333 """ - if kwargs: - warnings.warn( - "Function received unexpected arguments ({}) these " - "are ignored but will raise an Exception in the " - "future.".format(list(kwargs)), - AstropyDeprecationWarning) - if len(args) == 1 and callable(args[0]): return _custom_model_wrapper(args[0], fit_deriv=fit_deriv) elif not args: diff --git a/astropy/modeling/functional_models.py b/astropy/modeling/functional_models.py index 0b4530ec030..e3bf3fec4f4 100644 --- a/astropy/modeling/functional_models.py +++ b/astropy/modeling/functional_models.py @@ -6,7 +6,6 @@ from astropy import units as u from astropy.units import Quantity, UnitsError -from astropy.utils.decorators import deprecated from .core import (Fittable1DModel, Fittable2DModel) from .parameters import Parameter, InputParameterError @@ -2833,13 +2832,3 @@ def input_units(self): def _parameter_units_for_data_units(self, inputs_unit, outputs_unit): return {'tau': inputs_unit[self.inputs[0]], 'amplitude': outputs_unit[self.outputs[0]]} - - -@deprecated('4.0', alternative='RickerWavelet1D') -class MexicanHat1D(RickerWavelet1D): - """ Deprecated.""" - - -@deprecated('4.0', alternative='RickerWavelet2D') -class MexicanHat2D(RickerWavelet2D): - """ Deprecated.""" diff --git a/astropy/modeling/models.py b/astropy/modeling/models.py index cbe6ade8b3a..c3a2297b13b 100644 --- a/astropy/modeling/models.py +++ b/astropy/modeling/models.py @@ -17,9 +17,6 @@ from .tabular import * from . import math_functions as math -# Deprecated models that are not defined in __all__ -from .functional_models import MexicanHat1D, MexicanHat2D - # Attach a docstring explaining constraints to all models which support them. # Note: add new models to this list diff --git a/astropy/modeling/tests/test_functional_models.py b/astropy/modeling/tests/test_functional_models.py index bf1b479e0a2..0d23d05635c 100644 --- a/astropy/modeling/tests/test_functional_models.py +++ b/astropy/modeling/tests/test_functional_models.py @@ -8,7 +8,7 @@ from astropy.modeling import models, InputParameterError from astropy.coordinates import Angle from astropy.modeling import fitting -from astropy.utils.exceptions import AstropyDeprecationWarning, AstropyUserWarning +from astropy.utils.exceptions import AstropyUserWarning from astropy.utils.compat.optional_deps import HAS_SCIPY # noqa @@ -346,16 +346,3 @@ def test_ExponentialAndLogarithmic1D_fit(): log_model = models.Logarithmic1D(amplitude=1, tau=1) assert_allclose(xarr, em_model.inverse(em_model(xarr))) assert_allclose(xarr, log_model.inverse(log_model(xarr))) - - -def test_deprecated_hat_kernel(): - - # 'MexicanHat' was deprecated as a name for the models which are now - # 'RickerWavelet'. This test ensures that the models are correctly - # deprecated. - - with pytest.warns(AstropyDeprecationWarning): - models.MexicanHat1D() - - with pytest.warns(AstropyDeprecationWarning): - models.MexicanHat2D() diff --git a/astropy/modeling/tests/test_quantities_model.py b/astropy/modeling/tests/test_quantities_model.py index 99bc56f3c2d..cf28eb8be53 100644 --- a/astropy/modeling/tests/test_quantities_model.py +++ b/astropy/modeling/tests/test_quantities_model.py @@ -6,7 +6,6 @@ from astropy import units as u from astropy.tests.helper import assert_quantity_allclose -from astropy.utils.exceptions import AstropyDeprecationWarning from astropy.modeling.models import Mapping, Pix2Sky_TAN, Gaussian1D from astropy.modeling import models @@ -115,10 +114,7 @@ def _allmodels(): model = getattr(models, name) if type(model) is _ModelMeta: try: - with warnings.catch_warnings(): - # Ignore deprecation warnings from BlackBody1D and MexicanHat - warnings.simplefilter('ignore', category=AstropyDeprecationWarning) - m = model() + m = model() except Exception: pass allmodels.append(m) diff --git a/astropy/modeling/tests/test_utils.py b/astropy/modeling/tests/test_utils.py index 63525cb5ed5..f9dfde5564b 100644 --- a/astropy/modeling/tests/test_utils.py +++ b/astropy/modeling/tests/test_utils.py @@ -1,110 +1,10 @@ # Licensed under a 3-clause BSD style license - see LICENSE.rst # pylint: disable=invalid-name -import operator - -import numpy as np import pytest -from astropy.utils.exceptions import AstropyDeprecationWarning -from astropy.modeling.utils import ExpressionTree as ET, ellipse_extent -from astropy.modeling.models import Ellipse2D - from astropy.modeling.utils import _SpecialOperatorsDict -def test_traverse_postorder_duplicate_subtrees(): - """ - Regression test for a bug in `ExpressionTree.traverse_postorder` - where given an expression like ``(1 + 2) + (1 + 2)`` where the two proper - subtrees are actually the same object. - """ - with pytest.warns(AstropyDeprecationWarning): - subtree = ET('+', ET(1), ET(2)) - tree = ET('+', subtree, subtree) - traversal = [n.value for n in tree.traverse_postorder()] - assert traversal == [1, 2, '+', 1, 2, '+', '+'] - - -def test_tree_evaluate_subexpression(): - """Test evaluating a subexpression from an expression tree.""" - - operators = {'+': operator.add, '-': operator.sub, '*': operator.mul, - '/': operator.truediv, '**': operator.pow} - # The full expression represented by this tree is: - # 1.0 + 2 - 3 * 4 / 5 ** 6 (= 2.999232 if you must know) - with pytest.warns(AstropyDeprecationWarning): - tree = ET('+', ET(1.0), ET('-', ET(2.0), - ET('*', ET(3.0), ET('/', ET(4.0), - ET('**', ET(5.0), ET(6.0)))))) - - def test_slice(start, stop, expected): - assert np.allclose(tree.evaluate(operators, start=start, stop=stop), - expected) - - assert tree.evaluate(operators) == (1.0 + 2.0 - 3.0 * 4.0 / 5.0 ** 6.0) - test_slice(0, 5, (1.0 + 2.0 - 3.0 * 4.0 / 5.0)) - test_slice(0, 4, (1.0 + 2.0 - 3.0 * 4.0)) - test_slice(0, 3, (1.0 + 2.0 - 3.0)) - test_slice(0, 2, (1.0 + 2.0)) - test_slice(0, 1, 1.0) - - test_slice(1, 6, (2.0 - 3.0 * 4.0 / 5.0 ** 6.0)) - test_slice(1, 5, (2.0 - 3.0 * 4.0 / 5.0)) - test_slice(1, 4, (2.0 - 3.0 * 4.0)) - test_slice(1, 3, (2.0 - 3.0)) - test_slice(1, 2, 2.0) - - test_slice(2, 6, (3.0 * 4.0 / 5.0 ** 6.0)) - test_slice(2, 5, (3.0 * 4.0 / 5.0)) - test_slice(2, 4, (3.0 * 4.0)) - test_slice(2, 3, 3.0) - - test_slice(3, 6, (4.0 / 5.0 ** 6.0)) - test_slice(3, 5, (4.0 / 5.0)) - test_slice(3, 4, 4.0) - - test_slice(4, 6, (5.0 ** 6.0)) - test_slice(4, 5, 5.0) - - test_slice(5, 6, 6.0) - - -def test_ellipse_extent(): - # Test this properly bounds the ellipse - - imshape = (100, 100) - coords = y, x = np.indices(imshape) - - amplitude = 1 - x0 = 50 - y0 = 50 - a = 30 - b = 10 - theta = np.pi / 4 - - model = Ellipse2D(amplitude, x0, y0, a, b, theta) - - dx, dy = ellipse_extent(a, b, theta) - - limits = ((y0 - dy, y0 + dy), (x0 - dx, x0 + dx)) - - model.bounding_box = limits - - actual = model.render(coords=coords) - - expected = model(x, y) - - # Check that the full ellipse is captured - np.testing.assert_allclose(expected, actual, atol=0, rtol=1) - - # Check the bounding_box isn't too large - limits = np.array(limits).flatten() - for i in [0, 1]: - s = actual.sum(axis=i) - diff = np.abs(limits[2 * i] - np.where(s > 0)[0][0]) - assert diff < 1 - - def test__SpecialOperatorsDict__set_value(): key = 'test' val = 'value' diff --git a/astropy/modeling/utils.py b/astropy/modeling/utils.py index e507554e757..f41723fae55 100644 --- a/astropy/modeling/utils.py +++ b/astropy/modeling/utils.py @@ -4,295 +4,18 @@ This module provides utility functions for the models package. """ # pylint: disable=invalid-name -from collections import deque, UserDict +from collections import UserDict from collections.abc import MutableMapping from inspect import signature import numpy as np import warnings -from astropy.utils.decorators import deprecated -from astropy.utils import isiterable, check_broadcast +from astropy.utils import isiterable from astropy import units as u -__all__ = ['ExpressionTree', 'AliasDict', 'check_broadcast', - 'poly_map_domain', 'comb', 'ellipse_extent'] - -@deprecated('4.0') -class ExpressionTree: - __slots__ = ['left', 'right', 'value', 'inputs', 'outputs'] - - def __init__(self, value, left=None, right=None, inputs=None, outputs=None): - self.value = value - self.inputs = inputs - self.outputs = outputs - self.left = left - - # Two subtrees can't be the same *object* or else traverse_postorder - # breaks, so we just always copy the right subtree to subvert that. - if right is not None and left is right: - right = right.copy() - - self.right = right - - def __getstate__(self): - # For some reason the default pickle protocol on Python 2 does not just - # do this. On Python 3 it's not a problem. - return dict((slot, getattr(self, slot)) for slot in self.__slots__) - - def __setstate__(self, state): - for slot, value in state.items(): - setattr(self, slot, value) - - @staticmethod - def _recursive_lookup(branch, adict, key): - if isinstance(branch, ExpressionTree): - return adict[key] - return branch, key - - @property - def inputs_map(self): - """ - Map the names of the inputs to this ExpressionTree to the inputs to the leaf models. - """ - inputs_map = {} - if not isinstance(self.value, str): # If we don't have an operator the mapping is trivial - return {inp: (self.value, inp) for inp in self.inputs} - - elif self.value == '|': - l_inputs_map = self.left.inputs_map - for inp in self.inputs: - m, inp2 = self._recursive_lookup(self.left, l_inputs_map, inp) - inputs_map[inp] = m, inp2 - - elif self.value == '&': - l_inputs_map = self.left.inputs_map - r_inputs_map = self.right.inputs_map - for i, inp in enumerate(self.inputs): - if i < len(self.left.inputs): # Get from left - m, inp2 = self._recursive_lookup(self.left, - l_inputs_map, - self.left.inputs[i]) - inputs_map[inp] = m, inp2 - else: # Get from right - m, inp2 = self._recursive_lookup(self.right, - r_inputs_map, - self.right.inputs[i - len(self.left.inputs)]) - inputs_map[inp] = m, inp2 - - else: - l_inputs_map = self.left.inputs_map - for inp in self.left.inputs: - m, inp2 = self._recursive_lookup(self.left, l_inputs_map, inp) - inputs_map[inp] = m, inp2 - - return inputs_map - - @property - def outputs_map(self): - """ - Map the names of the outputs to this ExpressionTree to the outputs to the leaf models. - """ - outputs_map = {} - if not isinstance(self.value, str): # If we don't have an operator the mapping is trivial - return {out: (self.value, out) for out in self.outputs} - - elif self.value == '|': - r_outputs_map = self.right.outputs_map - for out in self.outputs: - m, out2 = self._recursive_lookup(self.right, r_outputs_map, out) - outputs_map[out] = m, out2 - - elif self.value == '&': - r_outputs_map = self.right.outputs_map - l_outputs_map = self.left.outputs_map - for i, out in enumerate(self.outputs): - if i < len(self.left.outputs): # Get from left - m, out2 = self._recursive_lookup(self.left, - l_outputs_map, - self.left.outputs[i]) - outputs_map[out] = m, out2 - else: # Get from right - m, out2 = self._recursive_lookup(self.right, - r_outputs_map, - self.right.outputs[i - len(self.left.outputs)]) - outputs_map[out] = m, out2 - - else: - l_outputs_map = self.left.outputs_map - for out in self.left.outputs: - m, out2 = self._recursive_lookup(self.left, l_outputs_map, out) - outputs_map[out] = m, out2 - - return outputs_map - - @property - def isleaf(self): - return self.left is None and self.right is None - - def traverse_preorder(self): - stack = deque([self]) - while stack: - node = stack.pop() - yield node - - if node.right is not None: - stack.append(node.right) - if node.left is not None: - stack.append(node.left) - - def traverse_inorder(self): - stack = deque() - node = self - while stack or node is not None: - if node is not None: - stack.append(node) - node = node.left - else: - node = stack.pop() - yield node - node = node.right - - def traverse_postorder(self): - stack = deque([self]) - last = None - while stack: - node = stack[-1] - if last is None or node is last.left or node is last.right: - if node.left is not None: - stack.append(node.left) - elif node.right is not None: - stack.append(node.right) - elif node.left is last and node.right is not None: - stack.append(node.right) - else: - yield stack.pop() - last = node - - def evaluate(self, operators, getter=None, start=0, stop=None): - """Evaluate the expression represented by this tree. - - ``Operators`` should be a dictionary mapping operator names ('tensor', - 'product', etc.) to a function that implements that operator for the - correct number of operands. - - If given, ``getter`` is a function evaluated on each *leaf* node's - value before applying the operator between them. This could be used, - for example, to operate on an attribute of the node values rather than - directly on the node values. The ``getter`` is passed both the index - of the leaf (a count starting at 0 that is incremented after each leaf - is found) and the leaf node itself. - - The ``start`` and ``stop`` arguments allow evaluating a sub-expression - within the expression tree. - - """ - - stack = deque() - - if getter is None: - getter = lambda idx, value: value - - if start is None: - start = 0 - - leaf_idx = 0 - for node in self.traverse_postorder(): - if node.isleaf: - # For a "tree" containing just a single operator at the root - # Also push the index of this leaf onto the stack, which will - # prove useful for evaluating subexpressions - stack.append((getter(leaf_idx, node.value), leaf_idx)) - leaf_idx += 1 - else: - operator = operators[node.value] - - if len(stack) < 2: - # Skip this operator if there are not enough operands on - # the stack; this can happen if some operands were skipped - # when evaluating a sub-expression - continue - - right = stack.pop() - left = stack.pop() - operands = [] - - for operand in (left, right): - # idx is the leaf index; -1 if not a leaf node - if operand[-1] == -1: - operands.append(operand) - else: - operand, idx = operand - if start <= idx and (stop is None or idx < stop): - operands.append((operand, idx)) - - if len(operands) == 2: - # evaluate the operator with the given operands and place - # the result on the stack (with -1 for the "leaf index" - # since this result is not a leaf node - left, right = operands - stack.append((operator(left[0], right[0]), -1)) - elif len(operands) == 0: - # Just push the left one back on the stack - # This is here because even if both operands were "skipped" - # due to being outside the (start, stop) range, we've only - # skipped one operator. But there should be at least 2 - # operators involving these operands, so we push the one - # from the left back onto the stack so that the next - # operator will be skipped as well. Should probably come - # up with an easier to follow way to write this algorithm - stack.append(left) - else: - # one or more of the operands was not included in the - # sub-expression slice, so don't evaluate the operator; - # instead place left over operands (if any) back on the - # stack for later use - stack.extend(operands) - - return stack.pop()[0] - - def copy(self): - # Hopefully this won't blow the stack for any practical case; if such a - # case arises that this won't work then I suppose we can find an - # iterative approach. - - children = [] - for child in (self.left, self.right): - if isinstance(child, ExpressionTree): - children.append(child.copy()) - else: - children.append(child) - - return self.__class__(self.value, left=children[0], right=children[1]) - - def format_expression(self, operator_precedence, format_leaf=None): - leaf_idx = 0 - operands = deque() - - if format_leaf is None: - format_leaf = lambda i, l: f'[{i}]' - - for node in self.traverse_postorder(): - if node.isleaf: - operands.append(format_leaf(leaf_idx, node)) - leaf_idx += 1 - continue - - oper_order = operator_precedence[node.value] - right = operands.pop() - left = operands.pop() - - if (node.left is not None and not node.left.isleaf and - operator_precedence[node.left.value] < oper_order): - left = f'({left})' - if (node.right is not None and not node.right.isleaf and - operator_precedence[node.right.value] < oper_order): - right = f'({right})' - - operands.append(' '.join((left, node.value, right))) - - return ''.join(operands) +__all__ = ['AliasDict', 'poly_map_domain', 'comb', 'ellipse_extent'] class AliasDict(MutableMapping): diff --git a/docs/changes/modeling/11978.api.rst b/docs/changes/modeling/11978.api.rst new file mode 100644 index 00000000000..3fc0932e24e --- /dev/null +++ b/docs/changes/modeling/11978.api.rst @@ -0,0 +1,7 @@ +Removed the following deprecated modeling features: + ``astropy.modeling.utils.ExpressionTree`` class, + ``astropy.modeling.functional_models.MexicanHat1D`` model, + ``astropy.modeling.functional_models.MexicanHat2D`` model, + ``astropy.modeling.core.Model.inputs`` setting in model initialize, + ``astropy.modeling.core.CompoundModel.inverse`` setting in model initialize, and + ``astropy.modeling.core.CompoundModel.both_inverses_exist()`` method.