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

Fixes Numpy datatype compatibility issues #8215

Merged
merged 2 commits into from
Feb 3, 2018
Merged

Fixes Numpy datatype compatibility issues #8215

merged 2 commits into from
Feb 3, 2018

Conversation

saurabhkgp21
Copy link
Contributor

Fixes #8207
Taking input as float16 datatype will give segmentation fault, since there are no corresponding types for it in C language. Here I am taking input as float64 type to avoid datatype incompatibility.

@ilayn ilayn added defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.ndimage labels Dec 13, 2017
@pv
Copy link
Member

pv commented Dec 13, 2017

This is not the right fix. The check for float16 needs to be done in the C code of ndimage.

@pv pv added the needs-work Items that are pending response from the author label Dec 13, 2017
@saurabhkgp21
Copy link
Contributor Author

@pv I will update this PR with float16 check in C code

@saurabhkgp21
Copy link
Contributor Author

saurabhkgp21 commented Dec 14, 2017

@pv please review and see if changes are required or not.

@ghost
Copy link

ghost commented Dec 14, 2017

IMO adding a test for this is trivial:

from scipy.ndimage.filters import gaussian_filter
import numpy as np
gaussian_filter(np.array([1],dtype=np.float16), 1)

@saurabhkgp21
Copy link
Contributor Author

Okay, I will add test cases for this one

@saurabhkgp21
Copy link
Contributor Author

@pv @xoviat I had added corresponding test cases, please check if everything is fine or not.

data = np.array([1],dtype = np.float16)
sigma = 1.0
with assert_raises(RuntimeError):
sndi.gaussian_filter(data,sigma)
Copy link

Choose a reason for hiding this comment

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

Nit: indent with four spaces instead of a tab.

@saurabhkgp21
Copy link
Contributor Author

ping @xoviat

@saurabhkgp21
Copy link
Contributor Author

@xoviat is this PR ready to be merged or some more work is needed?

@ghost ghost mentioned this pull request Feb 3, 2018
@pv pv removed the needs-work Items that are pending response from the author label Feb 3, 2018
@pv
Copy link
Member

pv commented Feb 3, 2018

I rebased this and replaced the magic number check array_type > 12 with an explicit list of types.

@pv pv merged commit cf14675 into scipy:master Feb 3, 2018
@pv
Copy link
Member

pv commented Feb 3, 2018

thanks, merged

@pv pv added the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Feb 3, 2018
@pv pv added this to the 1.0.1 milestone Feb 3, 2018
@saurabhkgp21 saurabhkgp21 deleted the dataype-compatibility branch February 17, 2018 04:58
@rgommers rgommers mentioned this pull request Mar 17, 2018
@rgommers rgommers removed the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Mar 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.ndimage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants