From 2a74ce47054c60470db810e932d1eb5bed32b73d Mon Sep 17 00:00:00 2001 From: Larry Bradley Date: Mon, 1 May 2023 18:42:02 -0400 Subject: [PATCH 1/4] Return nicer error message if factor is not integer --- astropy/convolution/tests/test_discretize.py | 7 +++++++ astropy/convolution/utils.py | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/astropy/convolution/tests/test_discretize.py b/astropy/convolution/tests/test_discretize.py index c75b5aece69..4d30bf787ba 100644 --- a/astropy/convolution/tests/test_discretize.py +++ b/astropy/convolution/tests/test_discretize.py @@ -265,3 +265,10 @@ def test_discretize_oversample(): assert_allclose(vmax, 0.927, atol=1e-3) assert vmax_yx == (25, 5) assert_allclose(values_center, values_osf1) + + +def test_oversample_factor(): + gauss_1D = Gaussian1D(1, 0, 0.1) + msg = "factor must have an integer value" + with pytest.raises(ValueError, match=msg): + discretize_model(gauss_1D, (-1, 2), mode="oversample", factor=1.2) diff --git a/astropy/convolution/utils.py b/astropy/convolution/utils.py index 5315609f4d6..777521bd89c 100644 --- a/astropy/convolution/utils.py +++ b/astropy/convolution/utils.py @@ -192,6 +192,10 @@ def discretize_model(model, x_range, y_range=None, mode="center", factor=10): " 'y_range' must be a whole number." ) + if factor != int(factor): + raise ValueError("factor must have an integer value") + factor = int(factor) + if ndim == 2 and y_range is None: raise ValueError("y_range must be specified for a 2D model") if ndim == 1 and y_range is not None: From 7f12fd6201ed0c4c6433f6aa5907c335aba80e49 Mon Sep 17 00:00:00 2001 From: Larry Bradley Date: Mon, 8 May 2023 14:59:51 -0400 Subject: [PATCH 2/4] Add changelog entry --- docs/changes/convolution/14794.api.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 docs/changes/convolution/14794.api.rst diff --git a/docs/changes/convolution/14794.api.rst b/docs/changes/convolution/14794.api.rst new file mode 100644 index 00000000000..5790ecbd183 --- /dev/null +++ b/docs/changes/convolution/14794.api.rst @@ -0,0 +1,2 @@ +``discretize_model`` will now raise a ``ValueError`` if +``mode='oversample'`` and ``factor`` does not have an integer value. From ce814ce5a40abd56e0346beda2848720e538b9c7 Mon Sep 17 00:00:00 2001 From: Larry Bradley Date: Mon, 8 May 2023 15:05:58 -0400 Subject: [PATCH 3/4] Improve the description of the oversample mode --- astropy/convolution/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/astropy/convolution/utils.py b/astropy/convolution/utils.py index 777521bd89c..ce6c1400706 100644 --- a/astropy/convolution/utils.py +++ b/astropy/convolution/utils.py @@ -120,7 +120,8 @@ def discretize_model(model, x_range, y_range=None, mode="center", factor=10): bins. For 2D models, the interpolation is bilinear. * ``'oversample'`` Discretize model by taking the average of model values - on an oversampled grid. + in the pixel bins on an oversampled grid. Use the + ``factor`` keyword to set the oversampling factor. * ``'integrate'`` Discretize model by integrating the model over the pixel bins using `scipy.integrate.quad`. This mode conserves From f74b879f683f7ef97065c03ee8ca87ccd85f18b8 Mon Sep 17 00:00:00 2001 From: Larry Bradley Date: Tue, 9 May 2023 10:58:06 -0400 Subject: [PATCH 4/4] Incorporate docstring suggestions --- astropy/convolution/utils.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/astropy/convolution/utils.py b/astropy/convolution/utils.py index ce6c1400706..d0f4d70253f 100644 --- a/astropy/convolution/utils.py +++ b/astropy/convolution/utils.py @@ -121,15 +121,16 @@ def discretize_model(model, x_range, y_range=None, mode="center", factor=10): * ``'oversample'`` Discretize model by taking the average of model values in the pixel bins on an oversampled grid. Use the - ``factor`` keyword to set the oversampling factor. + ``factor`` keyword to set the integer oversampling + factor. * ``'integrate'`` Discretize model by integrating the model over the pixel bins using `scipy.integrate.quad`. This mode conserves the model integral on a subpixel scale, but is very slow. factor : int, optional - The oversampling factor used when ``mode='oversample'``. Ignored - otherwise. + The integer oversampling factor used when ``mode='oversample'``. + Ignored otherwise. Returns -------