This one looks better already:)

@maniteja123 this segfaults on linux as well, so I assume you tested it and the fix works for you? If so, can you add the example of gh-5385 as a regression test? You can put it in numpy/core/tests/test_regression.py.

And like I commented on the other PR, just delete these two lines instead of commenting them out.

Side note: C++ style (//) comments are not supposed to be used in numpy, see the C style guide: https://github.com/numpy/numpy/blob/master/doc/C_STYLE_GUIDE.rst.txt#c-dialect

@rgommers Thanks for the advice. I have deleted the files and pushed it to the branch. Also, from next time I will make sure to follow the coding style guidelines. I was confused whether to delete the lines or not, so I commented them.

I have tested it by changing the file in inplace build and tested the example used in the issue #5354 to check whether seg fault is occurring or not . I have looked at the test_regressiontest.py file, but I am not particularly sure how to define the test functions and then assert statements. I have read the https://github.com/numpy/numpy/blob/master/doc/TESTS.rst.txt#writing-your-own-tests file, but how should I use the assert statements for the example in #5354 ?

You don't have to use an assert in this case. Just literally use the example that crashes now. If the issue is fixed the test will then pass, if not the whole test suite will crash.

I assume this TravisCI failure is unrelated?

Indeed looks unrelated. I've restarted the test.

Oh, then I will just add the corresponding program in a function and run numpy.test() ?

@argriffing So, should I add testing example to the test_multiarray.py file ? Also it looks like all PR related checks are added to test_regression.py file.

@maniteja123 I was wrong in that comment I'd deleted. There was a function in that file called test_clip but it was not actually testing clip. I agree that test_regression.py looks like a good place to put it.

Ran 5679 tests in 899.856s

OK (KNOWNFAIL=5, SKIP=8)
<nose.result.TextTestResult run=5679 errors=0 failures=0>

This is the result I got after running numpy.test()
Shall I also push the change I did in the test_regression.py ?

@maniteja123 you please push that change with the regression test.

And I'll have a look at what's going on with that USE_WHEEL test.

@rgommers I have pushed the change to the branch. I am not sure if this is what was expected output and also the test suite is correctly written. Please do have a look :)

Looks like a minor thing:

ERROR: Failure: TabError (inconsistent use of tabs and spaces in indentation (test_regression.py, line 2108))

You cannot mix tabs and spaces within Python files, it has to be spaces-only. You can replace them now by hand, but you should also look at setting up your text editor or IDE so it always replaces tabs with 4 spaces for all Python files. Any good editor will have an option to do this.

The USE_WHEEL failure is unrelated to this PR, and quite odd. The testlog should look like:

pip install --pre --upgrade --find-links dist numpy
Unpacking ./dist/numpy-1.10.0.dev4198570-cp27-none-linux_x86_64.whl
Installing collected packages: numpy
  Found existing installation: numpy 1.9.1
    Uninstalling numpy:
      Successfully uninstalled numpy
Successfully installed numpy

However for this PR it says:

pip install --pre --upgrade --find-links dist numpy
Requirement already up-to-date: numpy in /home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages
...
NPY_RELAXED_STRIDES_CHECKING set, but not active.

So somehow TravisCI is mixing up two builds.

Looks like pip version just changed from 1.5.6 to 6.0, so the failure with a wheel is likely a regression in`pip``.

@rgommers
As for the tabs, I have now manually used the 4 spaces and have pushed the changes. I am using Gedit right now. There was an option to insert spaces instead of tabs. I have put the tab width to 4.

Regarding travis build error, is there any chance of me using the inplace build causing the trouble there ?

Also where do I get to learn these commenting styles in the discussions on github like embedding links to source code (in particular, some lines in github repo ), highlighting key points or putting code snippets ?

pip version just changed from 1.5.6 to 6.0

Nice find. The earlier pip version bump also caused some complications if I remember correctly.

the regression test can be removed

the self.clip functions are wrapper functions around the real array clip used for the testing here, they do not have the same interface with default arguments, setting None explicitly is correct here then.

I gave a as np.ones(10, dtype=np.complex)
This is the failed test response. Please clarify.

FAIL: test_clip_complex (test_numeric.TestClip)

Traceback (most recent call last):
File "/home/maniteja/FOSS/numpy/numpy/core/tests/test_numeric.py", line 1187, in test_clip_complex
assert_array_strict_equal(ac, act)
File "/home/maniteja/FOSS/numpy/numpy/core/tests/test_numeric.py", line 1051, in assert_array_strict_equal
assert_array_equal(x, y)
File "/home/maniteja/FOSS/numpy/numpy/testing/utils.py", line 739, in assert_array_equal
verbose=verbose, header='Arrays are not equal')
File "/home/maniteja/FOSS/numpy/numpy/testing/utils.py", line 665, in assert_array_compare
raise AssertionError(msg)
AssertionError:
Arrays are not equal

(mismatch 100.0%)
x: array([ 1.+0.j, 1.+0.j, 1.+0.j, 1.+0.j, 1.+0.j, 1.+0.j, 1.+0.j,
1.+0.j, 1.+0.j, 1.+0.j])
y: array([None, None, None, None, None, None, None, None, None, None], dtype=object)

how does the test look like?
the assert means the result is not what you are expecting, either because of a bug in the test or the actual function tested is broken (which should not be the case)

def test_clip_complex(self):
    #Test native complex input without explicit min/max ie, either min=None or max=None
    #a   = 2 * self._generate_data_complex(self.nr, self.nc)
    a   = np.ones(10, dtype=np.complex)
    m   = a.min()
    M   = None
    ac  = self.fastclip(a, m, M)
    act = self.clip(a, m, M)
    assert_array_strict_equal(ac, act)      

I understand that it is a wrapper function. np.clip(a, a.min()) is raising a TypeError while a.clip(a.min()) isn't. But what I am not able to understand is why it is returning a ndarray of object type, while it works fine with np.clip or a.clip.
In the test output, the output we get by using the fastclip is correct, while the one using clip is incorrect

self.clip uses a different method to clip which apparently does not work with None
you can just put in the result you expect instead of calling self.clip

Oh, I see https://github.com/numpy/numpy/blob/master/numpy/core/tests/test_numeric.py#L1078 , which is a slow clip. I didn't think of that :)

Now, with a predefined example, the test suite is running fine. I will remove the regression test and push a commit to the branch

I have pushed the commit and the travis build also is successful :).

On a side note, would it be preferable to make the defined slow clip function to take care of the cases when min or max are specified as None ?

@maniteja123 another comment about style issues: almost every line contains a PEP8 violation:

numpy/core/tests/test_numeric.py:1180:9: E265 block comment should start with '# '
numpy/core/tests/test_numeric.py:1180:80: E501 line too long (91 > 79 characters)
numpy/core/tests/test_numeric.py:1181:10: E221 multiple spaces before operator
numpy/core/tests/test_numeric.py:1182:10: E221 multiple spaces before operator
numpy/core/tests/test_numeric.py:1183:10: E221 multiple spaces before operator
numpy/core/tests/test_numeric.py:1184:11: E221 multiple spaces before operator
numpy/core/tests/test_numeric.py:1185:25: E201 whitespace after '['
numpy/core/tests/test_numeric.py:1185:80: E501 line too long (113 > 79 characters)
numpy/core/tests/test_numeric.py:1186:43: W291 trailing whitespace
numpy/core/tests/test_numeric.py:1187:1: W293 blank line contains whitespace

You can run https://pypi.python.org/pypi/pep8 over your code and make sure it's clean. Note that it will report many other things in numpy which are there from long ago - but we try to not add new style problems....

@maniteja123 if you're comfortable with git, you may want to try to squash all commits (git rebase -i master and then change pick to squash in all but the first commit). If not, we'll do that before committing.

I think content-wise this can go in.

Sorry, I was out of station yesterday. I have taken care of PEP8 issues (I was unaware of the new rules, I just followed the previous guidelines ), also changed the comment for the regression test, mentioned the issue number 5354 and squashed the commits. I am not that acquainted with git, but am trying to get habituated to using git.
Just asking out of curiosity, doesn't these PEP issues show up in Travis build ?

Thanks, @maniteja123, in it goes.

Thanks @rgommers @jaimefrio @juliantaylor @argriffing, for the help and guidance. It was really good to learn a lot and contribute to numpy :)

@maniteja123 no problem. Keep them coming!

One last piece of github wisdom, @maniteja123: I closed issue #5354 manually, but had the commit message included something like fix #5354, it would have been closed automatically. See this to choose your favorite keyword.

@jaimefrio, thanks for the advise. It is a nice way of handling issues related to PR. I would take care to follow the convention next time :-)