fix numpy issue #3584

[abhijeetpanda12]

fixes #3584

line 731 in statsmodels/sandbox/stats/multicomp.py
midx = np.where(self.groupsunique==comparison_name)[0]
changed to
midx = np.where(self.groupsunique==comparison_name)[0][0]

[coveralls]

Coverage remained the same at 82.172% when pulling 70e739b on abhijeetpanda12:abhijeetpanda12-patch-1 into 523e3c9 on statsmodels:master.

[josef-pkt]

The tukeyhsd tests are in this module https://github.com/statsmodels/statsmodels/blob/master/statsmodels/stats/tests/test_pairwise.py

Based on my file search, there are currently no unit tests for the plot method.
You can add the call to plot_simultaneous to a current unit test class, or/and add a new test if the current unit test examples don't show this error.

specific to plot tests: matplotlib is not required, so the test needs to be skipped if matplotlib is not available. You can look at other plot test to see how this is done.

[abhijeetpanda12]

@josef-pkt Can you please check this sample code
which I feel should work for the plot_simultaneous method
https://gist.github.com/abhijeetpanda12/3017daed56e47a18b300ab18c89e756e

[josef-pkt]

Looks good,
One change use except ImportError, never catch all exceptions
Can you add this do an existing test, or do you need to add this as new class?
(I am offline for most of the day.)

[abhijeetpanda12]

@josef-pkt
Thanks for the advice.
I'll use except ImportError
I was thinking of adding it to that existing class CheckTuckeyHSDMixin
What are your views on this?

[josef-pkt]

I was thinking of adding it to that existing class CheckTuckeyHSDMixin

sounds good,
make sure that at least one test fails without the bug fix.

(Initially I thought we only add it to one of the test classes to keep test time lower, but it makes sense to check this for all cases, currently there are 5)

[abhijeetpanda12]

@josef-pkt
I am new to this codebase, can you help me out on what do you mean my
make sure that at least one test fails without the bug fix.
Shall I add it to all the 5 test classes present in
https://github.com/statsmodels/statsmodels/blob/master/statsmodels/stats/tests/test_pairwise.py

[josef-pkt]

You only need to add it to the CheckTuckeyHSDMixin class, as you said. Any test_xxx method there will be inherited and will be run in every test class.

make sure that at least one test fails without the bug fix

You already have the fix in this PR, but if you run the unit tests without this fix after adding the test to CheckTuckeyHSDMixin, then a test should fail. If no test fails, then the current case/bug needs an additional unit test

pytest (or nosetest) is convenient to run the unit tests of just one test module by specifying the module name, e.g. on the commandline
pytest <path of test_pairwise.py>

[abhijeetpanda12]

@josef-pkt
Yes, I had this in mind

You already have the fix in this PR, but if you run the unit tests without this fix after adding the test to CheckTuckeyHSDMixin, then a test should fail. If no test fails, then the current case/bug needs an additional unit test

But I think there won't be any test fails as there was no issue with the plot function, instead it was an issue with numpy.

I have attached the screenshot,
here I have added the test_plot_simultaneous() to the CheckTuckeyHSDMixin class without the numpy fix which is why there is an error while running the test.py file which contains the code where the plot_simultaneous() function fails.
The test still passes.

For that case, I have created a test class at
https://gist.github.com/abhijeetpanda12/b7e6b7850003e63ca1e0acefff8a81f5
Can you please check if it is correct?

[josef-pkt]

@abhijeetpanda12 Can you rebase this on current master? Otherwise, I take over the fix.
I added a unit test to the existing classes in #4380 which I expect will currently fail.

[josef-pkt]

this is now obselete, I added the fix to my PR to finish up
(we still count it as code sample for GSOC)

@abhijeetpanda12 Thanks for finding the bug-fix.
You can look at my PR to see what I meant above, first commit test to see a failure, then fix. test method is inherited by all subclasses, i.e. same test is applied to all test classes (currently 5)

[abhijeetpanda12]

@josef-pkt I was trying to rebase this on the current master. But you have already fixed it in #4380
What shall I do now?
I understood how you added the smoke test for the plot_simultaneous().

[josef-pkt]

@abhijeetpanda12 There is nothing for you to do anymore. I will close this PR when I merge my #4380
Thanks