From 23c37dee3649fc6d328246f101df5e43c114c197 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Wed, 8 Jan 2014 15:05:06 -0200 Subject: [PATCH 1/8] support showing subclassing deprecation warning only once --- scrapy/tests/test_utils_deprecate.py | 43 ++++++++++++++++++++++++++++ scrapy/utils/deprecate.py | 14 +++++++-- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/scrapy/tests/test_utils_deprecate.py b/scrapy/tests/test_utils_deprecate.py index 35aae8335..1d434512c 100644 --- a/scrapy/tests/test_utils_deprecate.py +++ b/scrapy/tests/test_utils_deprecate.py @@ -63,6 +63,49 @@ class WarnWhenSubclassedTest(unittest.TestCase): ) self.assertEqual(msg.lineno, lineno) + def test_warning_shown_everytime(self): + Deprecated = create_deprecated_class('Deprecated', NewName, + warn_category=MyWarning) + with warnings.catch_warnings(record=True) as w: + class U1(Deprecated): + pass + + class U2(Deprecated): + pass + + self.assertEqual(len(w), 2) + assert issubclass(w[0].category, MyWarning) + assert issubclass(w[1].category, MyWarning) + + with warnings.catch_warnings(record=True) as w: + _i1 = Deprecated() + _i2 = Deprecated() + + self.assertEqual(len(w), 2) + assert issubclass(w[0].category, MyWarning) + assert issubclass(w[1].category, MyWarning) + + def test_warning_shown_once(self): + Deprecated = create_deprecated_class('Deprecated', NewName, + warn_once=True, + warn_category=MyWarning) + with warnings.catch_warnings(record=True) as w: + class U1(Deprecated): + pass + + class U2(Deprecated): + pass + + self.assertEqual(len(w), 1) + assert issubclass(w[0].category, MyWarning) + + with warnings.catch_warnings(record=True) as w: + _i1 = Deprecated() + _i2 = Deprecated() + + self.assertEqual(len(w), 1) + assert issubclass(w[0].category, MyWarning) + def test_warning_auto_message(self): with warnings.catch_warnings(record=True) as w: Deprecated = create_deprecated_class('Deprecated', NewName) diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py index 20f4f1796..d4e5b835e 100644 --- a/scrapy/utils/deprecate.py +++ b/scrapy/utils/deprecate.py @@ -14,6 +14,7 @@ def attribute(obj, oldattr, newattr, version='0.12'): def create_deprecated_class(name, new_class, clsdict=None, warn_category=ScrapyDeprecationWarning, + warn_once=False, subclass_warn_message="{cls} inherits from "\ "deprecated class {old}, please inherit "\ "from {new}.", @@ -47,6 +48,8 @@ def create_deprecated_class(name, new_class, clsdict=None, class DeprecatedClass(type): deprecated_class = None + warned_on_subclass = False + warned_on_instance = False def __new__(metacls, name, bases, clsdict_): cls = super(DeprecatedClass, metacls).__new__(metacls, name, bases, clsdict_) @@ -55,8 +58,10 @@ def create_deprecated_class(name, new_class, clsdict=None, return cls def __init__(cls, name, bases, clsdict_): - old = cls.__class__.deprecated_class - if cls is not old: + meta = cls.__class__ + old = meta.deprecated_class + if (cls is not old) and not (warn_once and meta.warned_on_subclass): + meta.warned_on_subclass = True msg = subclass_warn_message.format(cls=_clspath(cls), old=_clspath(old), new=_clspath(new_class)) @@ -79,7 +84,10 @@ def create_deprecated_class(name, new_class, clsdict=None, return any(c in candidates for c in mro) def __call__(cls, *args, **kwargs): - if cls is cls.__class__.deprecated_class: + meta = cls.__class__ + old = meta.deprecated_class + if (cls is old) and not (warn_once and meta.warned_on_instance): + meta.warned_on_instance = True msg = instance_warn_message.format(cls=_clspath(cls), new=_clspath(new_class)) warnings.warn(msg, warn_category, stacklevel=2) From 8c81d6c6a9d9d69df975c7c3a80c8b6c7a05ef0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Fri, 10 Jan 2014 13:02:05 -0200 Subject: [PATCH 2/8] revert warning filter on deprecated class instantiation --- scrapy/tests/test_utils_deprecate.py | 8 +++++--- scrapy/utils/deprecate.py | 4 +--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/scrapy/tests/test_utils_deprecate.py b/scrapy/tests/test_utils_deprecate.py index 1d434512c..b255ca601 100644 --- a/scrapy/tests/test_utils_deprecate.py +++ b/scrapy/tests/test_utils_deprecate.py @@ -99,11 +99,13 @@ class WarnWhenSubclassedTest(unittest.TestCase): self.assertEqual(len(w), 1) assert issubclass(w[0].category, MyWarning) + # warns once instantations in the same lineno with warnings.catch_warnings(record=True) as w: - _i1 = Deprecated() - _i2 = Deprecated() + for _ in range(10): + _i1 = Deprecated() + _i2 = Deprecated() - self.assertEqual(len(w), 1) + self.assertEqual(len(w), 2) assert issubclass(w[0].category, MyWarning) def test_warning_auto_message(self): diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py index d4e5b835e..252a1eb53 100644 --- a/scrapy/utils/deprecate.py +++ b/scrapy/utils/deprecate.py @@ -49,7 +49,6 @@ def create_deprecated_class(name, new_class, clsdict=None, deprecated_class = None warned_on_subclass = False - warned_on_instance = False def __new__(metacls, name, bases, clsdict_): cls = super(DeprecatedClass, metacls).__new__(metacls, name, bases, clsdict_) @@ -86,8 +85,7 @@ def create_deprecated_class(name, new_class, clsdict=None, def __call__(cls, *args, **kwargs): meta = cls.__class__ old = meta.deprecated_class - if (cls is old) and not (warn_once and meta.warned_on_instance): - meta.warned_on_instance = True + if cls is old: msg = instance_warn_message.format(cls=_clspath(cls), new=_clspath(new_class)) warnings.warn(msg, warn_category, stacklevel=2) From a97b4aa6335622d86d0d2225594efa2021b330b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Fri, 10 Jan 2014 13:03:40 -0200 Subject: [PATCH 3/8] only warn on direct subclasses of the deprecated class --- scrapy/tests/test_utils_deprecate.py | 3 +++ scrapy/utils/deprecate.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/scrapy/tests/test_utils_deprecate.py b/scrapy/tests/test_utils_deprecate.py index b255ca601..377551d3a 100644 --- a/scrapy/tests/test_utils_deprecate.py +++ b/scrapy/tests/test_utils_deprecate.py @@ -31,6 +31,9 @@ class WarnWhenSubclassedTest(unittest.TestCase): class UserClass(Deprecated): pass + class NoWarnOnMe(UserClass): + pass + self.assertEqual(len(w), 1) msg = w[0] assert issubclass(msg.category, MyWarning) diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py index 252a1eb53..c9dac344a 100644 --- a/scrapy/utils/deprecate.py +++ b/scrapy/utils/deprecate.py @@ -59,7 +59,7 @@ def create_deprecated_class(name, new_class, clsdict=None, def __init__(cls, name, bases, clsdict_): meta = cls.__class__ old = meta.deprecated_class - if (cls is not old) and not (warn_once and meta.warned_on_subclass): + if old in bases and not (warn_once and meta.warned_on_subclass): meta.warned_on_subclass = True msg = subclass_warn_message.format(cls=_clspath(cls), old=_clspath(old), From 1e37b64ecda444e3337b724a2349d4b3d4d45f37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Fri, 10 Jan 2014 13:40:54 -0200 Subject: [PATCH 4/8] split subclassing tests and default to warning only once per deprecated class --- scrapy/tests/test_utils_deprecate.py | 131 +++++++++++++-------------- scrapy/utils/deprecate.py | 4 +- 2 files changed, 65 insertions(+), 70 deletions(-) diff --git a/scrapy/tests/test_utils_deprecate.py b/scrapy/tests/test_utils_deprecate.py index 377551d3a..6d18478df 100644 --- a/scrapy/tests/test_utils_deprecate.py +++ b/scrapy/tests/test_utils_deprecate.py @@ -17,99 +17,92 @@ class NewName(SomeBaseClass): class WarnWhenSubclassedTest(unittest.TestCase): + def _mywarnings(self, w, category=MyWarning): + return [x for x in w if x.category is MyWarning] + def test_no_warning_on_definition(self): with warnings.catch_warnings(record=True) as w: Deprecated = create_deprecated_class('Deprecated', NewName) + w = self._mywarnings(w) self.assertEqual(w, []) - def test_warning_on_subclassing(self): - with warnings.catch_warnings(record=True) as w: - Deprecated = create_deprecated_class('Deprecated', NewName, - warn_category=MyWarning) + def test_subclassing_warning_message(self): + Deprecated = create_deprecated_class('Deprecated', NewName, + warn_category=MyWarning) + with warnings.catch_warnings(record=True) as w: + class UserClass(Deprecated): + pass + + w = self._mywarnings(w) + self.assertEqual(len(w), 1) + self.assertEqual( + str(w[0].message), + "scrapy.tests.test_utils_deprecate.UserClass inherits from " + "deprecated class scrapy.tests.test_utils_deprecate.Deprecated, " + "please inherit from scrapy.tests.test_utils_deprecate.NewName." + " (warning only on first subclass, there may be others)" + ) + self.assertEqual(w[0].lineno, inspect.getsourcelines(UserClass)[1]) + + def test_subclassing_warns_only_on_direct_childs(self): + Deprecated = create_deprecated_class('Deprecated', NewName, + warn_once=False, + warn_category=MyWarning) + + with warnings.catch_warnings(record=True) as w: class UserClass(Deprecated): pass class NoWarnOnMe(UserClass): pass + w = self._mywarnings(w) self.assertEqual(len(w), 1) - msg = w[0] - assert issubclass(msg.category, MyWarning) - self.assertEqual( - str(msg.message), - "scrapy.tests.test_utils_deprecate.UserClass inherits from " - "deprecated class scrapy.tests.test_utils_deprecate.Deprecated, " - "please inherit from scrapy.tests.test_utils_deprecate.NewName." - ) - self.assertEqual(msg.lineno, inspect.getsourcelines(UserClass)[1]) + self.assertIn('UserClass', str(w[0].message)) + + def test_subclassing_warns_once_by_default(self): + Deprecated = create_deprecated_class('Deprecated', NewName, + warn_category=MyWarning) - def test_warning_on_instance(self): with warnings.catch_warnings(record=True) as w: - Deprecated = create_deprecated_class('Deprecated', NewName, - warn_category=MyWarning) - class UserClass(Deprecated): pass - _, lineno = Deprecated(), inspect.getlineno(inspect.currentframe()) - _ = UserClass() + class FooClass(Deprecated): + pass - self.assertEqual(len(w), 2) - msg = w[1] - assert issubclass(msg.category, MyWarning) + class BarClass(Deprecated): + pass + + w = self._mywarnings(w) + self.assertEqual(len(w), 1) + self.assertIn('UserClass', str(w[0].message)) + + def test_warning_on_instance(self): + Deprecated = create_deprecated_class('Deprecated', NewName, + warn_category=MyWarning) + + # ignore subclassing warnings + with warnings.catch_warnings(record=True): + class UserClass(Deprecated): + pass + + with warnings.catch_warnings(record=True) as w: + # warns only once on instantations in the same lineno + for _ in range(10): + _, lineno = Deprecated(), inspect.getlineno(inspect.currentframe()) + _ = UserClass() # subclass instances don't warn + + w = self._mywarnings(w) + self.assertEqual(len(w), 1) self.assertEqual( - str(msg.message), + str(w[0].message), "scrapy.tests.test_utils_deprecate.Deprecated is deprecated, " "instantiate scrapy.tests.test_utils_deprecate.NewName instead." ) - self.assertEqual(msg.lineno, lineno) - - def test_warning_shown_everytime(self): - Deprecated = create_deprecated_class('Deprecated', NewName, - warn_category=MyWarning) - with warnings.catch_warnings(record=True) as w: - class U1(Deprecated): - pass - - class U2(Deprecated): - pass - - self.assertEqual(len(w), 2) - assert issubclass(w[0].category, MyWarning) - assert issubclass(w[1].category, MyWarning) - - with warnings.catch_warnings(record=True) as w: - _i1 = Deprecated() - _i2 = Deprecated() - - self.assertEqual(len(w), 2) - assert issubclass(w[0].category, MyWarning) - assert issubclass(w[1].category, MyWarning) - - def test_warning_shown_once(self): - Deprecated = create_deprecated_class('Deprecated', NewName, - warn_once=True, - warn_category=MyWarning) - with warnings.catch_warnings(record=True) as w: - class U1(Deprecated): - pass - - class U2(Deprecated): - pass - - self.assertEqual(len(w), 1) - assert issubclass(w[0].category, MyWarning) - - # warns once instantations in the same lineno - with warnings.catch_warnings(record=True) as w: - for _ in range(10): - _i1 = Deprecated() - _i2 = Deprecated() - - self.assertEqual(len(w), 2) - assert issubclass(w[0].category, MyWarning) + self.assertEqual(w[0].lineno, lineno) def test_warning_auto_message(self): with warnings.catch_warnings(record=True) as w: diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py index c9dac344a..da3a4d2ee 100644 --- a/scrapy/utils/deprecate.py +++ b/scrapy/utils/deprecate.py @@ -14,7 +14,7 @@ def attribute(obj, oldattr, newattr, version='0.12'): def create_deprecated_class(name, new_class, clsdict=None, warn_category=ScrapyDeprecationWarning, - warn_once=False, + warn_once=True, subclass_warn_message="{cls} inherits from "\ "deprecated class {old}, please inherit "\ "from {new}.", @@ -64,6 +64,8 @@ def create_deprecated_class(name, new_class, clsdict=None, msg = subclass_warn_message.format(cls=_clspath(cls), old=_clspath(old), new=_clspath(new_class)) + if warn_once: + msg += ' (warning only on first subclass, there may be others)' warnings.warn(msg, warn_category, stacklevel=2) super(DeprecatedClass, cls).__init__(name, bases, clsdict_) From 6313b7ff4cf1b6df39fed2531682e1684e4e4023 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Fri, 10 Jan 2014 17:50:05 -0200 Subject: [PATCH 5/8] allow deprecation of a class with custom metaclass --- scrapy/tests/test_utils_deprecate.py | 5 +++++ scrapy/utils/deprecate.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/scrapy/tests/test_utils_deprecate.py b/scrapy/tests/test_utils_deprecate.py index 6d18478df..2afb780ef 100644 --- a/scrapy/tests/test_utils_deprecate.py +++ b/scrapy/tests/test_utils_deprecate.py @@ -177,3 +177,8 @@ class WarnWhenSubclassedTest(unittest.TestCase): Deprecated = create_deprecated_class('Deprecated', NewName, {'foo': 'bar'}) self.assertEqual(Deprecated.foo, 'bar') + + def test_deprecate_a_class_with_custom_metaclass(self): + Meta1 = type('Meta1', (type,), {}) + New = Meta1('New', (), {}) + Deprecated = create_deprecated_class('Deprecated', New) diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py index da3a4d2ee..6130ad800 100644 --- a/scrapy/utils/deprecate.py +++ b/scrapy/utils/deprecate.py @@ -45,7 +45,7 @@ def create_deprecated_class(name, new_class, clsdict=None, OldName. """ - class DeprecatedClass(type): + class DeprecatedClass(new_class.__class__): deprecated_class = None warned_on_subclass = False From 116f01f2d20602e58a9e98985f4a3a04252cb912 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Fri, 10 Jan 2014 16:44:54 -0200 Subject: [PATCH 6/8] allow overriding old and new class paths --- scrapy/tests/test_utils_deprecate.py | 19 +++++++++++++++++++ scrapy/utils/deprecate.py | 14 +++++++++----- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/scrapy/tests/test_utils_deprecate.py b/scrapy/tests/test_utils_deprecate.py index 2afb780ef..8cc3a26f1 100644 --- a/scrapy/tests/test_utils_deprecate.py +++ b/scrapy/tests/test_utils_deprecate.py @@ -46,6 +46,25 @@ class WarnWhenSubclassedTest(unittest.TestCase): ) self.assertEqual(w[0].lineno, inspect.getsourcelines(UserClass)[1]) + def test_custom_class_paths(self): + Deprecated = create_deprecated_class('Deprecated', NewName, + new_class_path='foo.NewClass', + old_class_path='bar.OldClass', + warn_category=MyWarning) + + with warnings.catch_warnings(record=True) as w: + class UserClass(Deprecated): + pass + + _ = Deprecated() + + w = self._mywarnings(w) + self.assertEqual(len(w), 2) + self.assertIn('foo.NewClass', str(w[0].message)) + self.assertIn('bar.OldClass', str(w[0].message)) + self.assertIn('foo.NewClass', str(w[1].message)) + self.assertIn('bar.OldClass', str(w[1].message)) + def test_subclassing_warns_only_on_direct_childs(self): Deprecated = create_deprecated_class('Deprecated', NewName, warn_once=False, diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py index 6130ad800..47d97e9cb 100644 --- a/scrapy/utils/deprecate.py +++ b/scrapy/utils/deprecate.py @@ -15,6 +15,8 @@ def attribute(obj, oldattr, newattr, version='0.12'): def create_deprecated_class(name, new_class, clsdict=None, warn_category=ScrapyDeprecationWarning, warn_once=True, + old_class_path=None, + new_class_path=None, subclass_warn_message="{cls} inherits from "\ "deprecated class {old}, please inherit "\ "from {new}.", @@ -62,8 +64,8 @@ def create_deprecated_class(name, new_class, clsdict=None, if old in bases and not (warn_once and meta.warned_on_subclass): meta.warned_on_subclass = True msg = subclass_warn_message.format(cls=_clspath(cls), - old=_clspath(old), - new=_clspath(new_class)) + old=_clspath(old, old_class_path), + new=_clspath(new_class, new_class_path)) if warn_once: msg += ' (warning only on first subclass, there may be others)' warnings.warn(msg, warn_category, stacklevel=2) @@ -88,8 +90,8 @@ def create_deprecated_class(name, new_class, clsdict=None, meta = cls.__class__ old = meta.deprecated_class if cls is old: - msg = instance_warn_message.format(cls=_clspath(cls), - new=_clspath(new_class)) + msg = instance_warn_message.format(cls=_clspath(cls, old_class_path), + new=_clspath(new_class, new_class_path)) warnings.warn(msg, warn_category, stacklevel=2) return super(DeprecatedClass, cls).__call__(*args, **kwargs) @@ -102,5 +104,7 @@ def create_deprecated_class(name, new_class, clsdict=None, return deprecated_cls -def _clspath(cls): +def _clspath(cls, forced=None): + if forced is not None: + return forced return '{}.{}'.format(cls.__module__, cls.__name__) From 31dd3dc4a1060e5e2ab053270e31529aa60819c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Fri, 10 Jan 2014 17:42:52 -0200 Subject: [PATCH 7/8] allow deprecation of subclasses of already deprecated classes --- scrapy/tests/test_utils_deprecate.py | 24 ++++++++++++++++++++++++ scrapy/utils/deprecate.py | 3 +-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/scrapy/tests/test_utils_deprecate.py b/scrapy/tests/test_utils_deprecate.py index 8cc3a26f1..75a53b2bf 100644 --- a/scrapy/tests/test_utils_deprecate.py +++ b/scrapy/tests/test_utils_deprecate.py @@ -201,3 +201,27 @@ class WarnWhenSubclassedTest(unittest.TestCase): Meta1 = type('Meta1', (type,), {}) New = Meta1('New', (), {}) Deprecated = create_deprecated_class('Deprecated', New) + + def test_deprecate_subclass_of_deprecated_class(self): + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter('always') + Deprecated = create_deprecated_class('Deprecated', NewName, + warn_category=MyWarning) + AlsoDeprecated = create_deprecated_class('AlsoDeprecated', Deprecated, + new_class_path='foo.Bar', + warn_category=MyWarning) + + w = self._mywarnings(w) + self.assertEqual(len(w), 0, str(map(str, w))) + + with warnings.catch_warnings(record=True) as w: + AlsoDeprecated() + class UserClass(AlsoDeprecated): + pass + + w = self._mywarnings(w) + self.assertEqual(len(w), 2) + self.assertIn('AlsoDeprecated', str(w[0].message)) + self.assertIn('foo.Bar', str(w[0].message)) + self.assertIn('AlsoDeprecated', str(w[1].message)) + self.assertIn('foo.Bar', str(w[1].message)) diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py index 47d97e9cb..edaecc3d3 100644 --- a/scrapy/utils/deprecate.py +++ b/scrapy/utils/deprecate.py @@ -87,8 +87,7 @@ def create_deprecated_class(name, new_class, clsdict=None, return any(c in candidates for c in mro) def __call__(cls, *args, **kwargs): - meta = cls.__class__ - old = meta.deprecated_class + old = DeprecatedClass.deprecated_class if cls is old: msg = instance_warn_message.format(cls=_clspath(cls, old_class_path), new=_clspath(new_class, new_class_path)) From 3e42646ce19edc54c82fd1a7caf38058452b51f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Thu, 16 Jan 2014 09:49:51 -0200 Subject: [PATCH 8/8] do not test multiple instantation warnings --- scrapy/tests/test_utils_deprecate.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/scrapy/tests/test_utils_deprecate.py b/scrapy/tests/test_utils_deprecate.py index 75a53b2bf..07b9a3dd5 100644 --- a/scrapy/tests/test_utils_deprecate.py +++ b/scrapy/tests/test_utils_deprecate.py @@ -109,10 +109,8 @@ class WarnWhenSubclassedTest(unittest.TestCase): pass with warnings.catch_warnings(record=True) as w: - # warns only once on instantations in the same lineno - for _ in range(10): - _, lineno = Deprecated(), inspect.getlineno(inspect.currentframe()) - _ = UserClass() # subclass instances don't warn + _, lineno = Deprecated(), inspect.getlineno(inspect.currentframe()) + _ = UserClass() # subclass instances don't warn w = self._mywarnings(w) self.assertEqual(len(w), 1)