Simplify deprecation machinery: don't cache previous messages, and use warnings rather than logging. #220
Conversation
…e warnings instead of logging.
|
I'd like to add tests for this. Those tests might be a bit messy, though: in Python 2, extra machinery is needed to reset the filterwarnings registry: |
|
Ah, the coverage change is probably related to the merge of #219. |
|
@itziakos: Would you be interested in reviewing? |
|
👍 |
|
Should probably add a test that checks the |
| # Lib/test/test_support.py) to reset the warnings registry | ||
| # for the module making use of this context manager. | ||
| # | ||
| # Note that this hack is unnecessary in Python 3.4 and later; see |
There was a problem hiding this comment.
Are we breaking compatibility with Python >= 3.4 by using this hack?
There was a problem hiding this comment.
No: it should be entirely harmless on Python >= 3.4: it clears out the __warningregistry__ dict, but on Python 3.4 the __warningregistry__ entries are effectively invalidated (but not removed) every time the warnings filterlist is touched [1], so all we're doing is removing entries from the registry that wouldn't have been used anyway.
The tests pass on Python 3.4, and we have Travis CI running on 3.4 too, so we should be pretty safe.
[1] There's a clever counter mechanism for this; see the Python issue for the details.
There was a problem hiding this comment.
We could add a sys.version check here, and not execute the hack for Python >= 3.4.
Looking closely, it's not quite true to say that it's unnecessary on Python >= 3.4: it's unnecessary on Python >= 3.4.2 (released in October). It's still necessary on 3.4.0 or 3.4.1, so it would be better not to execute the hack for Python >= 3.5. And Python 3.5 doesn't even exist yet.
All in all, I think I'd rather just leave this as it is. :-)
|
Other than the couple of comments above, looks good to me. |
|
Agreed that there should be a test for methods. I'll add one. |
|
Added a separate test module for (Note that we now have two test modules: one contains tests for |
|
Whoops; missing a |
… assert methods available on Python 2.6.
|
As @corranwebster pointed out, this also needs a changelog entry before it can be merged. |
|
LGTM - assuming Travis is happy with it. |
|
@corranwebster, @itziakos: Thanks for reviewing! Merging. |
Simplify deprecation machinery: don't cache previous messages, and use warnings rather than logging.
Alternative to #196.
This PR simplifies the Traits
@deprecateddecorator in two ways:deprecated#196).DeprecationWarning.If there's a strong case for using logging instead of warnings, we can revert the second part.
Updated the PR to add a test, along with an
assertDeprecatedmethod toUnittestTools.