PEP8 Cleanup, More Verbosness

michaelhelmick commented

2012-02-28 12:17:09 -08:00

(Migrated from github.com)

Rid of a lot of libs not being used
Changing Exceptions to prefix with "Twython", just safer in case
other apps have "AuthError", etc. for some reason.

- Rid of a lot of libs not being used - Changing Exceptions to prefix with "Twython", just safer in case other apps have "AuthError", etc. for some reason.

ryanmcgrath commented

2012-03-03 16:48:05 -08:00

(Migrated from github.com)

I'll merge this, as I think the Exception name changes are good, but the README (at the very least) should be updated to note that this changed. I'll probably do a larger release with these changes so it's evident that... well, something public-facing changed.

michaelhelmick commented

2012-03-05 05:32:17 -08:00

(Migrated from github.com)

Just wanted to let you know that the Exceptions names WERE changed without a version bump or README update. :P

okke-formsma commented

2012-03-30 02:44:48 -07:00

(Migrated from github.com)

Why does Twython create it's own errors such as TwythonError?

Many reusable errors are already defined in Python (http://docs.python.org/library/exceptions.html), so reusing those would be a good start.

Also, many exceptions are caught and then rethrown, which makes it much harder to debug issues. For example, in line 338:

        except RequestException, e:
            raise TwythonError("bulkUserLookup() failed with a %s error code." % e.code, e.code)

the Twython error masks the RequestException. In the stacktrace, it would have been clear that bulkUserLookup() threw the error, so information was actually lost!

Why does Twython create it's own errors such as TwythonError? Many reusable errors are already defined in Python (http://docs.python.org/library/exceptions.html), so reusing those would be a good start. Also, many exceptions are caught and then rethrown, which makes it much harder to debug issues. For example, in line 338: ``` python except RequestException, e: raise TwythonError("bulkUserLookup() failed with a %s error code." % e.code, e.code) ``` the Twython error masks the RequestException. In the stacktrace, it would have been clear that bulkUserLookup() threw the error, so information was actually lost!

michaelhelmick commented

2012-03-30 10:01:01 -07:00

(Migrated from github.com)

It's nice to know the error was brought up in the class, also; it's kind of just common practice requests or any other library.

It's nice to know the error was brought up in the class, also; it's kind of just common practice `requests` or any other library.

okke-formsma commented

2012-03-30 11:03:15 -07:00

(Migrated from github.com)

I always look at the stack trace to see where the error comes from; that way you can see where the error originally came from and also inspect values higher up in the stack (if you're using a debugger)

ryanmcgrath commented

2012-03-30 11:10:48 -07:00

(Migrated from github.com)

TwythonError could (and possibly should) be used for errors related to anything Twython-ish. There's any number of things that we could possibly want to catch and explicitly message about, which is why they exist. The code you've provided will throw an HTTP response error code if something's wrong - if not, chances are requests won't work anyway and you'll know that well before this point.

One way to meet in the middle on this issue would be to throw the rest of the stack trace with TwythonError - I'm open to a patch for this.

`TwythonError` could (and possibly should) be used for errors related to anything Twython-ish. There's any number of things that we could possibly want to catch and explicitly message about, which is why they exist. The code you've provided will throw an HTTP response error code if something's wrong - if not, chances are `requests` won't work anyway and you'll know that well before this point. One way to meet in the middle on this issue would be to throw the rest of the stack trace with TwythonError - I'm open to a patch for this.

Rows
Columns

PEP8 Cleanup, More Verbosness #62