Re: [Python-Dev] [Python-checkins] r66321 - in python/trunk: Doc/library/warnings.rst Lib/asynchat.py Lib/bsddb/test/test_early_close.py Lib/mimetools.py Lib/test/test___all__.py Lib/test/test_exceptions.py Lib/test/test_hmac.py Lib/test/test_import.py Lib/test/test_macostools.py Lib/test/test_pep352.py Lib/test/test_py3kwarn.py Lib/test/test_random.py Lib/test/test_re.py Lib/test/test_struct.py Lib/test/test_structmembers.py Lib/test/test_sundry.py Lib/test/test_support.py Lib/test/test_symtable.py Lib/tes

On 2008-09-09 02:49, brett.cannon wrote:
Author: brett.cannon Date: Tue Sep 9 02:49:16 2008 New Revision: 66321
Log: warnings.catch_warnings() now returns a list or None instead of the custom WarningsRecorder object. This makes the API simpler to use as no special object must be learned.
Closes issue 3781. Review by Benjamin Peterson.
This sounds a lot like a feature. Why was it added just before RC1 ?
Modified: python/trunk/Doc/library/warnings.rst python/trunk/Lib/asynchat.py python/trunk/Lib/bsddb/test/test_early_close.py python/trunk/Lib/mimetools.py python/trunk/Lib/test/test___all__.py python/trunk/Lib/test/test_exceptions.py python/trunk/Lib/test/test_hmac.py python/trunk/Lib/test/test_import.py python/trunk/Lib/test/test_macostools.py python/trunk/Lib/test/test_pep352.py python/trunk/Lib/test/test_py3kwarn.py python/trunk/Lib/test/test_random.py python/trunk/Lib/test/test_re.py python/trunk/Lib/test/test_struct.py python/trunk/Lib/test/test_structmembers.py python/trunk/Lib/test/test_sundry.py python/trunk/Lib/test/test_support.py python/trunk/Lib/test/test_symtable.py python/trunk/Lib/test/test_urllib.py python/trunk/Lib/test/test_urllibnet.py python/trunk/Lib/test/test_userstring.py python/trunk/Lib/test/test_warnings.py python/trunk/Lib/warnings.py python/trunk/Misc/NEWS
-- Marc-Andre Lemburg eGenix.com Professional Python Services directly from the Source (#1, Sep 09 2008)
Python/Zope Consulting and Support ... http://www.egenix.com/ mxODBC.Zope.Database.Adapter ... http://zope.egenix.com/ mxODBC, mxDateTime, mxTextTools ... http://python.egenix.com/
:::: Try mxODBC.Zope.DA for Windows,Linux,Solaris,MacOSX for free ! :::: eGenix.com Software, Skills and Services GmbH Pastor-Loeh-Str.48 D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg Registered at Amtsgericht Duesseldorf: HRB 46611

M.-A. Lemburg wrote:
On 2008-09-09 02:49, brett.cannon wrote:
Author: brett.cannon Date: Tue Sep 9 02:49:16 2008 New Revision: 66321
Log: warnings.catch_warnings() now returns a list or None instead of the custom WarningsRecorder object. This makes the API simpler to use as no special object must be learned.
Closes issue 3781. Review by Benjamin Peterson.
This sounds a lot like a feature.
Why was it added just before RC1 ?
It's also a bug that was introduced by the late API changes made to WarningsRecorder in r66135 (when WarningsRecorder was moved from test.test_support to warnings to make it officially supported for use outside the Python test suite, the API was also changed). Instead of changing the API *again* could we please get the API reverted back to the way it was for the two months prior to r66135 and only change the location from test_support to warnings? (obviously, the use of @contextmanager should still be removed since we don't particularly want warnings to depend on contextlib). Regards, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia --------------------------------------------------------------- http://www.boredomandlaziness.org

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Sep 9, 2008, at 6:31 AM, Nick Coghlan wrote:
It's also a bug that was introduced by the late API changes made to WarningsRecorder in r66135 (when WarningsRecorder was moved from test.test_support to warnings to make it officially supported for use outside the Python test suite, the API was also changed).
Instead of changing the API *again* could we please get the API reverted back to the way it was for the two months prior to r66135 and only change the location from test_support to warnings? (obviously, the use of @contextmanager should still be removed since we don't particularly want warnings to depend on contextlib).
As I commented in the bug, if this function is added to warnings I think it will be increasingly used as a way for code to suppress warnings. If that's the behavior we want to allow, then I think making every warning but the last inaccessible is a bug. If that's not a behavior we want to encourage then naming this function 'catch_warnings' and putting it in the warnings module sends the wrong message. Better to stick it in unittest if test_support is unacceptable. Or at least name it something that doesn't scream "use me in a way I don't intend!". - -Barry -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (Darwin) iQCVAwUBSMZrfHEjvBPtnXfVAQI6DgP8DyNKZyR846u1Qpgbr4X3wOM1J6EcqSlW 74WGUoU80MDWH5wvagAl7Er90oEh0QhX4ogX8UgF8iLSOC7jwa4CgpZsd9a5V8WB 8M0ia/G457vjGYg8XsaD5Dqvuv6NgMcW/2KJCJK3cuzEafan5C+I6n6lKQMAGOu0 j/9/PtIOKmI= =btg/ -----END PGP SIGNATURE-----

Barry Warsaw wrote:
On Sep 9, 2008, at 6:31 AM, Nick Coghlan wrote:
It's also a bug that was introduced by the late API changes made to WarningsRecorder in r66135 (when WarningsRecorder was moved from test.test_support to warnings to make it officially supported for use outside the Python test suite, the API was also changed).
Instead of changing the API *again* could we please get the API reverted back to the way it was for the two months prior to r66135 and only change the location from test_support to warnings? (obviously, the use of @contextmanager should still be removed since we don't particularly want warnings to depend on contextlib).
As I commented in the bug, if this function is added to warnings I think it will be increasingly used as a way for code to suppress warnings. If that's the behavior we want to allow, then I think making every warning but the last inaccessible is a bug.
If that's not a behavior we want to encourage then naming this function 'catch_warnings' and putting it in the warnings module sends the wrong message. Better to stick it in unittest if test_support is unacceptable. Or at least name it something that doesn't scream "use me in a way I don't intend!".
The last version of test_support.catch_warnings() actually did make all warnings caught available via the w.warnings attribute. The API and implementation changes in the move from test_support to the warnings module didn't actually add any new features - they just changed the w.warnings[idx] spelling to w[idx], moved from caching an explicit list of attributes on the WarningRecorder to a __getattr__ based lookup on the last warning and from a generator-based context manager to a custom class with __enter__ and __exit__ methods. The last was actually a good idea due to interpreter startup implications, but the first two changes were unnecessary (and the attribute handling change is actually what led to issue3781). Regardless, my preference at this point is for us to revert back to only providing this feature through the regression test suite, since that implementation at least worked properly, and reconsider the matter of proving a more public version through the warnings module for 2.7/3.1. Yes, it will make life a little more difficult for the twisted folks if they can't rely on our test support module being present for whatever reason, but they do have the option of including their own version of catch_warnings() as a fallback if the regression test suite can't be found. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia --------------------------------------------------------------- http://www.boredomandlaziness.org
participants (3)
-
Barry Warsaw
-
M.-A. Lemburg
-
Nick Coghlan