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

Raise nicer error message if factor is not integer #14794

Merged
merged 4 commits into from May 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions astropy/convolution/tests/test_discretize.py
Expand Up @@ -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)
12 changes: 9 additions & 3 deletions astropy/convolution/utils.py
Expand Up @@ -120,15 +120,17 @@
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 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
-------
Expand Down Expand Up @@ -192,6 +194,10 @@
" 'y_range' must be a whole number."
)

if factor != int(factor):
raise ValueError("factor must have an integer value")

Check warning on line 198 in astropy/convolution/utils.py

View check run for this annotation

Codecov / codecov/patch

astropy/convolution/utils.py#L198

Added line #L198 was not covered by tests
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:
Expand Down
2 changes: 2 additions & 0 deletions 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.