Update to coding standard

[mdruger]

Update code layout to AWeber coding standards.

The edits below are the changes I thought should be made. However, the entire code set should be reviewed to find areas that were missed.

[bkjones]

I did a cursory review only, of only the changes that were made. The nature of the card you're working on requires that the code review actually cover all of the entire files in the event that there were changes not made that should've been to bring the code base in line with our coding standards - @JohnBrodie or @chrismcguire or another stickler still needs to go over this. ;-)

[JohnBrodie]

These should be alphabetized.

[JohnBrodie]

@mdruger I'll do a more "holistic" review of this Monday, just wanted to get the ball rolling so you aren't blocked.

[JohnBrodie]

aweber_api/response.py:30:1: W391 blank line at end of file

[JohnBrodie]

There are a few spots in the code like aweber_api/collection.__init__ where we should be using super instead of the old-style method of calling parent methods.

[JohnBrodie]

In aweber_api/collection (and likely other places) we do:

import aweber_api
from aweber_api.response import AWeberResponse

We generally try to be as specific as possible with the imports.

[JohnBrodie]

aweber_api/__init__.py has imports in _read_response. They should be moved to the top of the file, assuming they aren't placed there for circular import reasons...

[JohnBrodie]

I'm not sure if this is out of scope, but since you're touching the whole codebase, I'll mention it. There are a bunch of "todo"/"we should test this"/"find a better way to do this" in the code, it'd be great if we could make decisions on the comments one way or the other, act on those decisions, and remove the extraneous comments.

[JohnBrodie]

@mdruger That's all I got.

[JohnBrodie]

Tests fail locally, and on Travis https://travis-ci.org/aweber/AWeber-API-Python-Library/pull_requests

[JohnBrodie]

Not sure how this is passing on Travis - the Mock package is being imported, but isn't in setup.py test_requires section. python setup.py develop && python setup.py nosetests fails in a clean virtualenv.

miked: ok, for this changeset, I will keep dingus. When the tests are updated, I will replace dingus.

[JohnBrodie]

This is going to blow up L99 with a TypeError.

[mdruger]

I will rewrite this function

[JohnBrodie]

For what it's worth, the commits to update to using Mock didn't need ripped out, Mock just needed added to setup.py. If you'd prefer to do that in a different PR, that's fine though.

[JohnBrodie]

We usually catch specific exceptions, not the base Exception type.

[mdruger]

I'll catch the AttributeError

[JohnBrodie]

From flake8:

aweber_api/__init__.py:11: W402 'AWeberCollection' imported but unused
aweber_api/__init__.py:12: W402 'AWeberEntry' imported but unused
aweber_api/__init__.py:14: W402 'AWeberResponse' imported but unused
aweber_api/collection.py:4: W402 'collections' imported but unused
aweber_api/collection.py:130:38: E225 missing whitespace around operator
aweber_api/collection.py:134:28: E225 missing whitespace around operator

miked: I addressed the collections and missing whitespace issues. Thanks for verifying the imported and unused modules with me.

[JohnBrodie]

This is less than ideal. We are taking apart the URL above, and then very slowly and oddly reassembling it, creating a bunch of extra string objects along the way, and throwing in some magic numbers for good measure... Is there not a URL parsing lib we could be using here?

[mdruger]

I agree. The trouble here is that the variable 'url' is a misnomer in this case. We aren't actually manipulating an entire url string that includes the protocol, netloc, path, and all the other parameters that urlparse determines. The url looks something like this '/accounts/1/lists/1'. As awesome as urlparse is, using it here only returns the equivalent path string which has to use the split method to tokenize the string with the '/' delimiter (i.e. url_parts = url.split('/')). I will try a different string manipulation method that hopefully, will be easier to follow. What is even more confusing is that the string manipulation will change depending on whether the host class is an entry or a collection. I found away to place this in the base class, by adding a parameter to acknowledge the different class.

[JohnBrodie]

Single quotes preferred over double quotes.