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

Remove warnings supression as noted by Issue #229 #231

Merged
merged 2 commits into from
Nov 10, 2016
Merged

Conversation

mmcky
Copy link
Contributor

@mmcky mmcky commented Feb 25, 2016

This PR removes the warning supression from arma.py and matrix_eqn.py.

TODO: The next step is to fix the underlying cause of the warning.

@mmcky
Copy link
Contributor Author

mmcky commented Feb 25, 2016

The arma warnings are a result of:

w, spect = self.spectral_density(two_pi=False)

the spect array is complex. With the example on the quant-econ.net page the coeeficient on the j term are all zero. In general is it safe to use the real component of this array? If so we can add a.real on spec to send matplotlib an array of reals.

@mmcky
Copy link
Contributor Author

mmcky commented Mar 1, 2016

Moving related comments here from Issue #229

Comment in arma.py

# == Ignore unnecessary warnings concerning casting complex variables back to
# floats == #

@jstac In general is it safe to use the real component of this array?

@mmcky
Copy link
Contributor Author

mmcky commented Mar 1, 2016

Comment in matrix_eqn.py

# == Suppress warnings from checking conditioning of matrices == #

@mmcky mmcky mentioned this pull request Mar 1, 2016
@oyamad oyamad mentioned this pull request Apr 6, 2016
2 tasks
@mmcky
Copy link
Contributor Author

mmcky commented Jul 25, 2016

@jstac would you mind reviewing this spectral density question when you have time? Its in relation to some warnings we are trying to remove in the quantecon.py package.

@oyamad
Copy link
Member

oyamad commented Nov 9, 2016

What is the status of this PR?

@mmcky
Copy link
Contributor Author

mmcky commented Nov 10, 2016

this PR has removed the instructions to ignore and suppress warnings. So the last step is to review arma.py and matrix_eqn.py to resolve the root cause of the issued warnings.

@oyamad
Copy link
Member

oyamad commented Nov 10, 2016

Is it possible to merge this PR as is and open another issue for discussion on arma.py and matrix_eqn.py? I find it a bit annoying that warnings are all ignored in the whole session once quantecon is imported...

@mmcky
Copy link
Contributor Author

mmcky commented Nov 10, 2016

Sure. I will have a quick review of this this afternoon to double check that all of the suppression statements have been removed and then I will setup a separate issue to fix arma.py and matrix_eqn.py.

@oyamad
Copy link
Member

oyamad commented Nov 10, 2016

Thanks!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 86.633% when pulling 0302277 on adjust-warnings into 712cae6 on master.

@mmcky
Copy link
Contributor Author

mmcky commented Nov 10, 2016

This looks ready to merge (passes tests).

Note: this will cause warnings to appear when using the library.

#271
#272

@mmcky mmcky merged commit e9e3214 into master Nov 10, 2016
@mmcky mmcky deleted the adjust-warnings branch November 10, 2016 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants