berkerpeksag / astor

Python AST read/write
https://pypi.org/project/astor/
BSD 3-Clause "New" or "Revised" License
810 stars 102 forks source link

Spring cleaning #33

Closed pmaupin closed 9 years ago

pmaupin commented 9 years ago

I tried to tease out some of the refactoring from some of what I am adding.

I have not yet updated the docs in this branch -- first I wanted to make sure you are OK with the general direction.

Thanks, Pat

berkerpeksag commented 9 years ago

Just did a quick review and looks great to me :)

Two things:

import warnings

def oldname(foo, bar):
    warnings.warn("oldname is deprecated.", DeprecationWarning, stacklevel=2)
    return new_name(foo, bar)

instead of

oldname = new_name

But I can do that before releasing 0.6.

pmaupin commented 9 years ago

I actually played with deprecation warnings, but then realized they don't show up by default, so I don't know how useful they really are. I guess it depends on if people actually use the -w switch.

I think the right way to do the deprecation warnings might be to build a decorator function so we can do something like foo = warnold(bar)

I'm working on the doc now.

berkerpeksag commented 9 years ago

Yes, the unittest module (and other test frameworks) sets -Wd by default. So, when you run the tests, you'll see all warnings.

pmaupin commented 9 years ago

So, when you run the tests, you'll see all warnings.

That makes sense. I've done all I can do on the docs for right now. If you think it's a good start, should you go ahead and merge it into the master and deal with the deprecations later, or is there some subtlety I'm missing in the process?

Thanks, Pat

berkerpeksag commented 9 years ago

LGTM. I'll deal with deprecations later, thanks!

pmaupin commented 9 years ago

Thanks!

Forgot to mention -- I have no heartburn dropping 3.2 (never used) or 2.6 from the testing, etc., but if we break something on one of these and somebody reports a simple patch that will make it work again, I have no heartburn accepting that, either...