deprecate Accessor and related classes in twisted.python.reflect

[jesstess]

Specifically: Settable, AccessorType, PropertyAccessor, Accessor, and Summer.

They are old and unused. Accessor is only for pre-Python 2.2 code.

[moijes12]

Hi, I'm having a problem here.

I have deprecated the metaclass twisted.python.reflect.AccessorType and the class twisted.python.reflect.PropertyAccessor. I was able to write the unite test for Accessor but with PropertyAccessor,I am facing a problem.The line of code in the function that causes the problem is propertyAccessor = reflect.AccessorType(,(PropertyAccessorTest(),),{}). It gives error function() argument 1 must be code, not str. Please can someone suggest a way out.

[moijes12]

This is a demo patch. I'm having some trouble deprecating PropertyAccessorType and Accessor.

[moijes12]

Hi. I've sent in a demo patch. I'm not sure if this violates any policy but if it does I apologize for it. Please provide some advice on how to deprecate PropertyAccessorType and Accessor especially since they are all related with AccessorType being the metaclass and Summer being the child class of Accessor.

[exarkun]

Thanks for your work on this!

The patch doesn't appear to demonstrate any problem, so I don't know if I can really answer your question. I can make a guess, that when you use the deprecated decorator on the definition of Accessor, problems arise - and attempting this, I see the error you mentioned, so I probably guessed right. It's better if you provide a patch (or otherwise complete, self-contained example) that demonstrates the problem you encountered though, so no guessing is required. :)

As for the problem, it's caused by the fact that deprecated is not a valid class decorator. It is only a function decorator. It always returns a function, and so the Summer subclass fails, since you cannot subclass a function.

Try using twisted.python.deprecate.deprecatedModuleAttribute for all of these deprecations, instead. It works on any attribute (including global variables defined using =) and also works on all versions of Python supported by Twisted (class decorators are not supported by Python 2.5, which Twisted still supports).

[moijes12]

Settable, AccessorType, PropertyAccessor, Accessor, OriginalAccessor and Summer are deprecated ( version used is 12.1.0) . Unit Tests are created in test_reflect.py and news files entry is made in twisted/topfiles

[moijes12]

Please use this patch and not the previous one for review.

[jesstess]

(In [33556]) Branching to 'deprecate-accessor-5451'

[jesstess]

Thanks for working on this, moijes12! Your changes look good. A few comments:

• In all cases, deprecatedModuleAttribute should say why the class is getting deprecated. You could say something about how Accessor is for Python 2.1 and earlier, which we no longer support, and that these classes are old and untested.
• Please wrap the deprecatedModuleAttribute lines after 79 characters, as described in the  coding standard.
• While exercising these deprecated classes in test_reflect.py, you make some unused assignments that pyflakes warns about. Can you remove the assignments and avoid these warning? :

$ pyflakes twisted/test/test_reflect.py
twisted/test/test_reflect.py:789: local variable 'settable' is assigned to but never used
twisted/test/test_reflect.py:798: local variable 'accessorType' is assigned to but never used
twisted/test/test_reflect.py:807: local variable 'accessorType' is assigned to but never used
twisted/test/test_reflect.py:816: local variable 'accessorType' is assigned to but never used
twisted/test/test_reflect.py:825: 'OriginalAccessor' imported but unused
twisted/test/test_reflect.py:834: local variable 'accessorType' is assigned to but never used

You might find a helper method like lookForDeprecationWarning in twisted/internet/test/test_gtkreactor.py helpful for factoring out the duplicate code in these tests.

I created a branch for this ticket but didn't end up committing anything to it, so please submit a revised patch against trunk.

Thanks again!

[moijes12]

Required changes are mage

[moijes12]

Required changes have been made.

[jesstess]

(In [33558]) Apply patch Twisted_5451.patch by moijes12 with some cosmetic changes.

refs #5451

[jesstess]

Thanks for the revised patch, moijes12. I committed your patch to the branch with a couple of mostly cosmetic changes:

• AccessorType and friends aren't only for Python 2.2, they are for Python 2.2 and up (they are new-style classes), so I adjusted the deprecation message to reflect that.
• I removed some trailing whitespace.
• I'm not sure what text editor you are using, but the way you were wrapping lines isn't the way that emacs or the rest of the file wraps lines. For example, your patch had:

+deprecatedModuleAttribute(Version("Twisted", 12, 1, 0),
+"OriginalAccessor is a reference to class twisted.python.reflect.Accessor "
+"which is deprecated.", "twisted.python.reflect", "OriginalAccessor")

with the continued lines flush with the first line, instead of indented, e.g.:

+deprecatedModuleAttribute(
+    Version("Twisted", 12, 1, 0),
+    "OriginalAccessor is a reference to class twisted.python.reflect.Accessor "
+    "which is deprecated.", "twisted.python.reflect", "OriginalAccessor")

Other than that,  build results look good, so I'll go ahead and merge!

[jesstess]

(In [33559]) Merge deprecate-accessor-5451

Author: moijes12 Reviewer: jesstess Fixes: #5451

Deprecate twisted.python.reflect's Settable, AccessorType, PropertyAccessor, Accessor, OriginalAccessor and Summer classes. Accessor is Python 2.1 and earlier only, and all are unused and untested.