-
Notifications
You must be signed in to change notification settings - Fork 136
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
exception_level_filters: support string for filtered class name #38
Conversation
Couple of general comments:
|
@@ -403,7 +403,16 @@ def prev_page(self): | |||
|
|||
|
|||
def _filtered_level(exception): | |||
for cls, level in SETTINGS['exception_level_filters']: | |||
for i, item in enumerate(SETTINGS['exception_level_filters']): | |||
cls, level = item |
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.
I think it might be a good idea to break the body of the for
loop out into its own function. Otherwise the continue
part of the logic is easy to overlook / a little hard to reason about (had me confused for a bit).
Thanks for the PR! I think the behavior as you implemented it is good. See a few comments above. |
… import module 'exceptions' in unit test because it breaks compatibility with Python3
Looks good to me. @coryvirok can you review this as well? |
Hey @coryvirok, do you think you will review and merge sometime this week? |
@asnyatkov I'm on it |
@@ -401,10 +401,23 @@ def prev_page(self): | |||
|
|||
## internal functions | |||
|
|||
def _resolve_exception_class(idx, filter): | |||
cls, level = filter | |||
if isinstance(cls, str): |
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.
I'd use is instance(cls, basestring)
- http://stackoverflow.com/questions/1979004/what-is-the-difference-between-isinstanceaaa-basestring-and-isinstanceaaa
This looks great! I'll merge once the Thanks! |
I'll just make the change myself so we can get this out. |
exception_level_filters: support string for filtered class name
Super! Thanks! |
Alrighty, 0.9.3 is released on pypi |
#37
Added ability to specify exception level filters using either string or class type.
Added unit test for exception_level_filters