twisted.python.deprecate.deprecated gets the name of methods wrong

Description

If you deprecate a method x on a class Y in the module z, twisted.python.deprecate.deprecated thinks that the FQPN of the thing that it's decorating is z.x and reports it as such, not reporting the fairly interesting detail of the class Y.

Change History (23)

I'd really like to update the compatibility policy to recommend this decorator instead of ad-hoc warnings.warn calls, so that we can have a systematic approach to deprecation and consistent error messages, but this seems like a blocker for doing that, so I'll work on it this weekend.

I sincerely apologize. :( I was working on deprecating something that was difficult without this decorator working, so I did some work on this issue on a plane. I should have checked before I got on the plane whether there was a ticket and work outstanding.

Thanks tons for your work on this Julian! I hope it's ok that I uploaded the branch?

This branch passes the python 2.7 and python 3.3 tests locally (I still need to kick off buildbot).

classmethods and staticmethods don't work though. I think classmethods may involve fixing _fullyQualifiedName, but I'm not sure that static methods can work. :|

but deprecated will not give them to you. I wonder what deprecated *is* good for, actually.

Hmm. Good point.

The work we did in this branch (woo pair programming in cramped airplane seats) gets a little closer to @deprecated being good for something. It allows you to deprecate methods, which deprecatedModuleAttribute doesn't let you do. However, it still emits the warning at call time rather than at __get__ time which, as you point out, is probably wrong.

What if we adjusted @deprecated's behavior to always do what deprecatedModuleAttribute does when called at module scope?

It allows you to deprecate methods, which deprecatedModuleAttribute doesn't let you do.

Ah yes, thank you for pointing that out.

What if we adjusted @deprecated's behavior to always do what deprecatedModuleAttribute does when called at module scope?

I think that sounds pretty cool. It would be nice if we could say deprecated is the thing to use for anything you can decorate. We'll still need deprecatedModuleAttribute for other stuff but I think classes, functions, and methods are the things we deprecate by far most often.

This is probably all fodder for follow-up tickets, though. The change being proposed here still makes sense as a self-contained improvement.

I see an error when I put @deprecated above @classmethod.
AttributeError: 'classmethod' object has no attribute '__module__'.
In trunk I see a different error.
AttributeError: 'classmethod' object has no attribute '__name__'.
However, if @deprecated comes below, the deprecation warning is emitted.

If I supply a reference to a classmethod as the replacement parameter, the message doesn't render the actual FQDN of the replacement.
Instead I get the message please use <classmethod object at 0xd84868> instead

twisted/python/deprecate.py

Typo "Actally"

Various missing full-stops after sentences in docstrings and epydoc fields.

_type could be given a more descriptive name...maybe deprecatee_type

Typo "caled"

_DeprecateDescriptor

I think the convention is to document the initialiser parameters and then refer to those in the @ivar epydocs.

I think the convention is to document the initialiser parameters and then refer to those in the @ivar epydocs.

There has been some extended and rather diffuse discussion on this topic, so I will try to give some background. Really, this needs to be in the coding standard somewhere and not just repeated on tickets but at least let me try to get the right stuff repeated :-).

When we started requiring epydoc fields everywhere, this resulted in a tremendous amount of duplication of process where people would write a ton of prose for @param in __init__, then get review feedback that they forgot to document an @ivar, and go copy and paste that. So there was a lot of redundancy and the opportunity for things to get de-synchronized.

So we started saying instead that if a value is both a @param and an @ivar, and they have the same name, you only need to document it in one place or the other, because clearly more documentation than that is just busy-work. (Unfortunately this is presented horribly in pydoctor, as the __init__ still shows up as undocumented.) Since a public attribute is a longer-lived aspect of an API than a constructor argument, we generally preferred to document @ivars over @params when choosing one or the other.

This unfortunately lead to some tickets including copious documentation of private@ivar attributes, but no documentation of the public@params for __init__. That's obviously bad, so we started instead saying that you should document things in @param.

So policy has shifted considerably over time, and where we are now is something like:

twistedchecker is going to complain if you don't document every @ivar and every @param, and we want each new commit to be twistedchecker-clean on the buildbot. So you have to document all of them.

aside: I tend to object to workarounds less than exarkun does, but generally speaking workarounds which just make you write more informative documentation that is presented better are less onerous than things which, for example, distort your code to make it less readable to make it pass pyflakes.

Since attributes are more frequently private these days than they used to be, it is good form to document the parameters to __init__ first, since that is the user's entry-point into the API. However, since public attributes are important to document (and because of the aforementioned twistedchecker issue), we now include @ivar descriptions, even if they're redundant; but to make them less redundant, we refer to __init__ and @param.

However, it is possible to actually L{}-link directly to an @ivar, but not possible to L{}-link to a @param as far as I know, so if there are public attributes that are intended to be used externally as attributes and not just constructor arguments then it may make sense to do the reverse and link from the @param to the @ivar.

There should really be a ticket to update the coding standard to lay this out cleanly.

There has been some extended and rather diffuse discussion on this topic, so I will try to give some background. Really, this needs to be in the coding standard somewhere and not just repeated on tickets but at least let me try to get the right stuff repeated :-).

There should really be a ticket to update the coding standard to lay this out cleanly.

We seem to be going the direction of "document every possible thing" these days. This stems from good intentions but the result is ever more bloated developer documentation of which any individual contributor has an ever shrinking knowledge. Rather than continuing to block the docs even further (though we don't have to do so in this particular case since it happened already, hooray) I think we need to get serious about pursuing a different strategy - for example, making twistedchecker a piece of software we could actually maintain and the output of which we could actually rely on (this really is just an example - the notion of a tool that tells you every single thing that's wrong with a piece of software is, of course, quite enticing - but perhaps unachievable).