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.

Sign up for GitHub

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

Jump to bottom

Added normalized_epsilon as a parameter to convolve_fft #5177

Closed
wants to merge 2 commits into from
Closed

Added normalized_epsilon as a parameter to convolve_fft #5177

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f3b9a6
Added normalized_epsilon as a parameter to convolve_fft
samverstocken Jul 14, 2016
7422363
Adjusted description of normalized_epsilon parameter
samverstocken Jul 14, 2016
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
9 changes: 6 additions & 3 deletions astropy/convolution/convolve.py
View file
Open in desktop
Expand Up @@ -226,8 +226,8 @@ def convolve(array, kernel, boundary='fill', fill_value=0.,
def convolve_fft(array, kernel, boundary='fill', fill_value=0, crop=True,
return_fft=False, fft_pad=None, psf_pad=None,
interpolate_nan=False, quiet=False, ignore_edge_zeros=False,
min_wt=0.0, normalize_kernel=False, allow_huge=False,
fftn=np.fft.fftn, ifftn=np.fft.ifftn,
min_wt=0.0, normalize_kernel=False, normalized_epsilon=1e-8,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line contains a trailing whitespace (at the end of the line). That's why your first build failied during the PEP-8 check.

allow_huge=False, fftn=np.fft.fftn, ifftn=np.fft.ifftn,
complex_dtype=np.complex):
"""
Convolve an ndarray with an nd-kernel. Returns a convolved image with
Expand Down Expand Up @@ -312,6 +312,9 @@ def convolve_fft(array, kernel, boundary='fill', fill_value=0, crop=True,
256.
quiet : bool, optional
Silence warning message about NaN interpolation
normalized_epsilon: float, optional
The upper limit on the deviation of the kernel sum with respect to unity.
Will be checked only if normalize_kernel is False.
Copy link
Member

@pllim pllim Jul 14, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add `` around normalize_kernel to mark it as another keyword.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if a parameter is optional it is custom to provide or hint at the default value. Something like Default is "1e-8". (or defaults to xxx, ...)

Another thing that could be added is a sphinx directive when the parameter was added, something like: .. versionadded:: 1.3 (or 1.2.x).

This reminds me: This PR needs a Changelog entry (home directory /Changes.rst) for the appropriate version/subpackage under "API changes" (or "New features").

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more of an API change.

allow_huge : bool, optional
Allow huge arrays in the FFT? If False, will raise an exception if the
array or kernel size is >1 GB
Expand Down Expand Up @@ -440,7 +443,7 @@ def convolve_fft(array, kernel, boundary='fill', fill_value=0, crop=True,
kernel = kernel / normalize_kernel(kernel)
kernel_is_normalized = True
else:
if np.abs(kernel.sum() - 1) < 1e-8:
if np.abs(kernel.sum() - 1) < normalized_epsilon:
kernel_is_normalized = True
else:
kernel_is_normalized = False
Expand Down