[FIX] Fix Future Warnings in ivp.py and test_quad.py and RuntimeError in lq_control.py.

[duncanhobbs]

This PR fixes some warnings raised in issues #505 #506 and #507.

1. FutureWarning in ivp.py (see issue [Tests] Fix Pandas related FutureWarnings #505).

2. RuntimeError in lq_control.py (see issue [Tests] Fix RuntimeWarning in lqcontrol.py #506)

3. Pandas related future warnings in test_quad.py (see issue [Tests] Fix FutureWarning in ivp.py #507).

This is my first QuantEcon PR so let me know if I need to fix anything.

[mmcky]

thanks @duncanhobbs -- this will be helpful. I will review this in the next couple of days.

[oyamad]

Thanks @duncanhobbs

In lq_control.py, beta = 1 should be rejected if C != 0 in this branch

QuantEcon.py/quantecon/lqcontrol.py

Lines 143 to 146 in d9c1e8a

	else:
	self.P = None
	self.d = None
	self.T = None

by something like:

if (C != 0).any() and beta >= 1:
    raise ValueError('beta must be strictly smaller than 1')

[duncanhobbs]

Thanks @oyamad.

I adjusted the code to incorporate what you suggested, and am going through the other files that use LQ to make sure the other tests do not break.

In test_lqnash.py here it raises ValueError 'beta must be strictly smaller than 1'.

I think this is because beta = 1 and C = None by default.

Should the ValueError be thrown if C = None?

Or only if C != 0 or C is not None?

[oyamad]

@duncanhobbs Sorry my code was wrong. C != 0 should have been self.C != 0.
And it will be better to provide more information in the error message.

Try this:

if (self.C != 0).any() and beta >= 1:
    raise ValueError('beta must be strictly smaller than 1' +
                     'when T is None and C != 0')

[duncanhobbs]

Thanks @oyamad

I made the tweak and it solved the issue in test_lqnash.py.

When I ran the test suite again, the ValueError was raised in test_robust_lq.py here.

If I assign beta = beta and theta = theta the tests pass.

Is that okay to do?

[oyamad]

The problem is in this line:

QuantEcon.py/quantecon/tests/test_robustlq.py

Line 51 in d9c1e8a

self.lq_test = LQ(Q, R, A, B, C, beta)

beta is an optional argument, so it has to be passed as beta=beta:

        self.lq_test = LQ(Q, R, A, B, C, beta=beta)

(See also QuantEcon/lecture-source-py#314)

[pep8speaks]

Hello @duncanhobbs! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file quantecon/lqcontrol.py:

Line 150:21: E128 continuation line under-indented for visual indent

Comment last updated at 2019-09-19 01:40:35 UTC

[coveralls]

Coverage decreased (-0.01%) to 94.085% when pulling ae592db on duncanhobbs:fix-tests into d9c1e8a on QuantEcon:master.

[duncanhobbs]

Hi @oyamad

I have made the changes you requested.

I am not sure why the other commits got added from July 11. It may have happened when I was trying to rebase and clean up the commit history. I forgot to rebase my branch before pushing commits to the pull request.

Would the easiest way to fix this be to rebase the pull request commits or open a new PR with the feature branch updated?

It feels to me that opening a new PR is easier, but I am not sure what standard practice is.

Apologies.

[oyamad]

Are the changes in these two lines really necessary?

[duncanhobbs]

I see they are not. I will fix this.

[oyamad]

Thanks @duncanhobbs
Can you rebase your branch on the current master (and then force-push)?

[duncanhobbs]

Thanks @oyamad.

I rebased and force-pushed as you suggested and things seem to work.

The tests all pass locally.

Thanks for your patience. I am not very experienced with using git yet.

Let me know if I need to change anything else.

[oyamad]

@duncanhobbs Thank you for your contribution. Much appreciated!

[jstac]

Thanks @duncanhobbs, much appreciated.

[mmcky]

thanks @duncanhobbs and @oyamad for your reviews and advice on this.