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

TST: increase scipy.optimize test coverage for disp=True #4805

Merged
merged 3 commits into from May 18, 2015

Conversation

argriffing
Copy link
Contributor

This includes a new decorator in _lib._testutils for silencing output of specific tests, inspired by https://github.com/numpy/numpy/blob/master/numpy/testing/decorators.py and https://github.com/numpy/numpy/blob/maintenance/1.9.x/numpy/lib/tests/test_regression.py#L171, and it reorganizes the optimization tests by adding an object oriented hierarchy inspired by the scipy.sparse test framework, replacing

    def test_minimize(self):
        # Tests for the minimize wrapper.
        self.setUp()
        self.test_bfgs(True)
        self.setUp()
        self.test_bfgs_infinite(True)
        self.setUp()
        self.test_cg(True)
        self.setUp()
        self.test_ncg(True)
        self.setUp()
        self.test_ncg_hess(True)
        self.setUp()
        self.test_ncg_hessp(True)
        self.setUp()
        self.test_neldermead(True)
        self.setUp()
        self.test_powell(True)
        self.setUp()
        self.test_custom()

closes #4804

@pv
Copy link
Member

pv commented May 4, 2015 via email

@ewmoore
Copy link
Member

ewmoore commented May 4, 2015

os.devnull should be fine on windows.

On Mon, May 4, 2015 at 12:49 PM, Pauli Virtanen notifications@github.com
wrote:

os.devnull on windows?


Reply to this email directly or view it on GitHub
#4805 (comment).

@argriffing
Copy link
Contributor Author

os.devnull on windows?

numpy uses it...

Edit: it exists on windows according to https://docs.python.org/2/library/os.html#os.devnull

@@ -25,3 +26,19 @@ def deco(func):
pass
return dec.knownfailureif(True, msg)(func)
return deco


def suppressed_stdout():
Copy link
Member

Choose a reason for hiding this comment

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

What's the point of this surrounding function if the decorator does not accept arguments? Unless I'm missing something, it could just be the underlying function suppressed_stdout_decorator, which would be used simply as @suppressed_stdout (no parentheses).

@argriffing
Copy link
Contributor Author

@dlax Thanks for your comment, I've simplified the decorator. I was afraid that if I didn't make it look like an airport then planes wouldn't land with cargo.

@rgommers
Copy link
Member

This is tricky to get right. I think stdout.close() doesn't do what you'd expect, see numpy/numpy@3fd6b62c1db915

Also see this context manager for redirecting stdout which may somehow be useful/informative: numpy/numpy#3128

@rgommers rgommers added the maintenance Items related to regular maintenance tasks label May 17, 2015
@rgommers
Copy link
Member

+1 for this idea though. We may want to add this decorator to numpy.testing once it's been proven to be robust (i.e. no complaints for a month or so after this has been merged).

@argriffing
Copy link
Contributor Author

I don't know how to progress further with this PR, but I've opened a numpy issue numpy/numpy#5891.

@rgommers
Copy link
Member

It's late and I haven't thought about this very hard, but I think your code is actually OK. sys.stdout.close() doesn't work for the original sys.stdout, but you're only doing this on your dummy stdout so should work.

@rgommers
Copy link
Member

If my comment above makes sense, and this PR works without generating any noise, maybe just merge it. Any issues have about 5 months until the next release to surface.

argriffing added a commit that referenced this pull request May 18, 2015
TST: increase scipy.optimize test coverage for disp=True
@argriffing argriffing merged commit 3610c9e into scipy:master May 18, 2015
@argriffing
Copy link
Contributor Author

OK I've merged it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Items related to regular maintenance tasks scipy.optimize
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add scipy.optimize.minimize smoke tests for disp=True
6 participants