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

Warn only when n in not int in tauchen #673

Merged
merged 3 commits into from
Jan 3, 2023
Merged

Conversation

Smit-create
Copy link
Member

@coveralls
Copy link

coveralls commented Dec 19, 2022

Coverage Status

Coverage increased (+0.02%) to 94.053% when pulling fa87ffd on Smit-create:c1 into b1c0cdf on QuantEcon:main.

" https://github.com/QuantEcon/QuantEcon.py/issues/663.",
UserWarning, stacklevel=2)

if not isinstance(n, int):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if not isinstance(n, int):
if not isinstance(n, numbers.Integral):

isinstance(n, int) returns False for example if n is an np.int_:

>>> n = np.array([1])[0]
>>> type(n)
<class 'numpy.int64'>
>>> isinstance(n, int)
False

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, fixed!

@@ -19,8 +19,7 @@ def setup_method(self):
self.tol = 1e-12
self.mu = 0.

with pytest.warns(UserWarning):
mc = tauchen(self.n, self.rho, self.sigma, self.mu, self.n_std)
mc = tauchen(self.n, self.rho, self.sigma, self.mu, self.n_std)
Copy link
Contributor

Choose a reason for hiding this comment

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

@Smit-create maybe we can also test the warning when the old interface is used?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Thanks @mmcky

@mmcky
Copy link
Contributor

mmcky commented Dec 20, 2022

thanks @Smit-create I think this is a good approach 👍

@mmcky mmcky self-requested a review December 20, 2022 04:03
@Smit-create Smit-create merged commit 147d862 into QuantEcon:main Jan 3, 2023
@Smit-create Smit-create deleted the c1 branch January 3, 2023 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants