Issue 340760043: [plaso] Added deprecated decorator (Closed)
|
Created:
6 years, 2 months ago by Joachim Metz Modified:
6 years ago CC:
kiddi, log2timeline-dev_googlegroups.com Visibility:
Public. |
Description[plaso] Added deprecated decorator
Patch Set 1 #
Total comments: 6
Patch Set 2 : Changes after review #MessagesTotal messages: 14
https://codereview.appspot.com/340760043/diff/1/plaso/lib/decorators.py File plaso/lib/decorators.py (right): https://codereview.appspot.com/340760043/diff/1/plaso/lib/decorators.py#newco... plaso/lib/decorators.py:19: warnings.simplefilter('default', DeprecationWarning) I'm not well versed into warnings module, why do you change the 'always' filter to the 'default' one?
Sign in to reply to this message.
https://codereview.appspot.com/340760043/diff/1/plaso/lib/decorators.py File plaso/lib/decorators.py (right): https://codereview.appspot.com/340760043/diff/1/plaso/lib/decorators.py#newco... plaso/lib/decorators.py:19: warnings.simplefilter('default', DeprecationWarning) DeprecationWarning is ignored by default, see: https://docs.python.org/3/library/warnings.html If I recall correctly, setting to "always" will turn on the working and then changing to default will "print the first occurrence of matching warnings for each location where the warning is issued" https://docs.python.org/3/library/warnings.html#warning-filter
Sign in to reply to this message.
https://codereview.appspot.com/340760043/diff/1/plaso/lib/decorators.py File plaso/lib/decorators.py (right): https://codereview.appspot.com/340760043/diff/1/plaso/lib/decorators.py#newco... plaso/lib/decorators.py:19: warnings.simplefilter('default', DeprecationWarning) On 2018/02/22 13:41:39, Joachim Metz wrote: > DeprecationWarning is ignored by default, see: > https://docs.python.org/3/library/warnings.html > > If I recall correctly, setting to "always" will turn on the working and then > changing to default will "print the first occurrence of matching warnings for > each location where the warning is issued" > > https://docs.python.org/3/library/warnings.html#warning-filter Testing a bit locally (py2 & py3): - This will be ignored: warnings.warn("deprecated", DeprecationWarning) - This will be displayed: warnings.simplefilter('default', DeprecationWarning) warnings.warn("deprecated", DeprecationWarning) So I think you can set the 'default' filter once here, which ignores the default setting. You can also use the 'once' filter which unlike 'default' will ensure the warning displays only once. This sentence was confusing to write.
Sign in to reply to this message.
ack, though the warning needs to be shown once per call
Sign in to reply to this message.
https://codereview.appspot.com/340760043/diff/1/plaso/lib/decorators.py File plaso/lib/decorators.py (right): https://codereview.appspot.com/340760043/diff/1/plaso/lib/decorators.py#newco... plaso/lib/decorators.py:19: warnings.simplefilter('default', DeprecationWarning) ack, though the warning needs to be shown once per call
Sign in to reply to this message.
https://codereview.appspot.com/340760043/diff/1/plaso/lib/decorators.py File plaso/lib/decorators.py (right): https://codereview.appspot.com/340760043/diff/1/plaso/lib/decorators.py#newco... plaso/lib/decorators.py:19: warnings.simplefilter('default', DeprecationWarning) On 2018/02/22 15:39:36, Joachim Metz wrote: > ack, though the warning needs to be shown once per call Does it need to be shown per call though? If this is an loop somewhere, it's going to cause a bunch of spam that's not helpful to the user.
Sign in to reply to this message.
only the "first time per call", so that you know where in the code the function needs to be replaced so per loop it is only shown once, but shown every where it is used
Sign in to reply to this message.
https://codereview.appspot.com/340760043/diff/1/plaso/lib/decorators.py File plaso/lib/decorators.py (right): https://codereview.appspot.com/340760043/diff/1/plaso/lib/decorators.py#newco... plaso/lib/decorators.py:19: warnings.simplefilter('default', DeprecationWarning) only the "first time per call", so that you know where in the code the function needs to be replaced so per loop it is only shown once, but shown every where it is used
Sign in to reply to this message.
the main reason I could find is the filter order matters: http://blog.zsoldosp.eu/2016/02/01/warnings-warn-some-deprecationwarning-gotc... however for our purpose the default filter seems to suffice for now so I'll update it and we can always change it back it needed
Sign in to reply to this message.
Code updated.
Sign in to reply to this message.
Changes have been merged with master branch. To close the review and clean up the feature branch you can run: review.py close decorators
Sign in to reply to this message.
|
