-
Notifications
You must be signed in to change notification settings - Fork 68
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
Warnings verbosity #1213
base: master
Are you sure you want to change the base?
Warnings verbosity #1213
Changes from all commits
ff26f31
3412bab
0009530
58cfa69
7011b5e
1fe6f07
e61a252
5ddbeb5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,50 @@ | ||
__all__ = ("CCLError", "CCLWarning", "CCLDeprecationWarning",) | ||
__all__ = ("CCLError", "CCLWarning", "CCLDeprecationWarning", | ||
"warnings", "update_warning_verbosity") | ||
|
||
import warnings | ||
import warnings as warnings_builtin | ||
|
||
|
||
_warning_importance = {'high': 10, 'low': 1} | ||
_verbosity_thresholds = {'none': 100, 'low': 10, 'high': 1} | ||
|
||
|
||
class warnings: | ||
_CCL_WARN_THRESHOLD = 10 # Equivalent to "low" verbosity | ||
|
||
def warn(*args, **kwargs): | ||
category = kwargs.get('category') | ||
importance = _warning_importance[kwargs.pop('importance', 'low')] | ||
|
||
if ((category in (CCLWarning, CCLDeprecationWarning)) and | ||
(importance < warnings._CCL_WARN_THRESHOLD)): | ||
return | ||
|
||
warnings_builtin.warn(*args, **kwargs) | ||
|
||
|
||
def update_warning_verbosity(verbosity): | ||
""" Update the level of verbosity of the CCL warnings. Available | ||
levels are "none", "low", and "high". More warning messages will | ||
be output for higher verbosity levels. If "none", no CCL-level | ||
warnings will be shown. The default verbosity is "low". Note that | ||
unless the verbosity level is "high", all C-level warnings will | ||
be omitted. | ||
|
||
Args: | ||
verbosity (str): one of ``'none'``, ``'low'`` or ``'high'``. | ||
""" | ||
|
||
if not (verbosity in ['none', 'low', 'high']): | ||
raise KeyError("`verbosity` must be one of {'none', 'low', 'high'}") | ||
warnings._CCL_WARN_THRESHOLD = _verbosity_thresholds[verbosity] | ||
|
||
# Remove C-level warnings | ||
from . import debug_mode | ||
|
||
if verbosity == 'high': | ||
debug_mode(True) | ||
else: | ||
debug_mode(False) | ||
|
||
|
||
class CCLError(RuntimeError): | ||
|
@@ -40,8 +84,8 @@ def __hash__(self): | |
|
||
@classmethod | ||
def enable(cls): | ||
warnings.simplefilter("always") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are the deprecation warnings covered by the verbosity filter as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so, but let me check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No they are not. Do you think they should? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My gut feeling is yes - they're good to know while building a model but you probably don't want them in an MCMC log. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If they mute them and CCL's deprecation cycle completes, their MCMC will simply stop working, and they won't be able to figure out why. I don't think it's a good idea muting those. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, but deprecation warnings have to do with CCL functionality whereas CCLWarnings mostly have to do with numerical stability, so they represent different things. At the very least I'd add a different flag to separate the verbosity level for deprecation-specific warnings. BTW now that v3 is out, is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, and also, deprecation warnings should be on my default, contrary to other, informative-only warnings, because that's how you tell people that the API will change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CCLDeprecationWarnings are not used right now. I'll leave them there for the future in any case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They seemed to be used here: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As developers we can decide what the priority of the different warnings are. If we decide that specific deprecation warnings are sufficiently important, we can just tag them as "high", and they'll go through the filter. @tilmantroester I've now added the CCLDeprecationWarnings to the list of warnings that can be suppressed if desired. |
||
warnings_builtin.simplefilter("always") | ||
|
||
@classmethod | ||
def disable(cls): | ||
warnings.filterwarnings(action="ignore", category=cls) | ||
warnings_builtin.filterwarnings(action="ignore", category=cls) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Start reviewing here.