Make the code PEP8 compliant.

[jensrudolf]

Hi @noseka1,

I really like your project Linuxband and do want contribute at little to keep it alive.

This commit tries to make the code more PEP8 compliant by fixing most of the reports generated by the flake8 tool. Contrary to PEP8, I set the maximum line length to a more reasonable value of 120 characters.

There are some whitespace reportings left, especially regarding the file chord_table.py. However, this is intended to keep the chords tables readable and totally consistent with PEP20 (The Zen of Python).

This work is preparing a possible port to Python 3 as Python 2 is close to EOL.

The following changes have been made:

• Cleanup unnecessary blank lines
• Add blank lines where necessary
• Use 'is' and 'is not' operator for comparisons to None
• Split the use of multiple statements on one line (E701)
• Remove semicola from statements (E703)
• Remove unnecessary whitespaces (E201, E202)
• Add whitespaces where necessary
• Remove some useless inline comments
• Replace '<>' with '!=' (W603)
• Remove redundant backslash (E502)
• Remove mixed indention with tabstops and whitespace
• Sort module imports according to PEP8
• Remove line break before binary operator (W503)
• Fix ambigous variable naming (E741)
• Fix usage of bare 'except' (E722)
• Replace escape sequences with corresponding utf-8 symbol (W605)
• Improve indention and alignment to increase readability
• Fix wrong comment indention
• Align line continuations with visual indent
• Reduce length of some lines to keep em < 120 chars

Feedback is greatly appreciated.

[jensrudolf]

I refactored the code to omit the "unused variables" by unpacking the tuples or using the underscore character, where necessary. Furthermore, I reworked the exception handling to catch the exceptions that can be thrown according to the std lib's documentation. Additionally, the exception message is appended to the logging output in order to ease debugging. There is still room for improvement. However, porting the code to Python 3 eventually might simplify certain cases (e.g. by using context managers).

What do you think about it?

[jensrudolf]

Hi @noseka1, did you find some time to look into the changes of my latest commit 5fe2743? What do you think about the exception handling?

[jensrudolf]

Sorry, the close was unintentionally. I hit the wrong button :-(

[noseka1]

I was thinking about the exception handling and reviewed your latest approach. I like that you drop e.strerror to the logs.

I still don't feel comfortable about limiting the scope of caught exceptions, i.e. changing

except:
  # all exception handled here

to

except ConfigParser.Error as e:
  # not really all possible exceptions handled here

If it would work for you, I think we could do:

except Exception as e:
  # almost all exceptions handled here

[noseka1]

One more comment regarding the size of this pull request. There is many code changes included on this pull request. These are valuable code changes that I would like to get into the master branch, however, it's difficult to review and merge everything at once.

I suggest to break this pull request up into multiple pull requests. For each item on the "The following changes have been made:" list there could be a separate pull request. That will allow for easier review and also much clearer commit history. What do you think?

[jensrudolf]

Regarding the exception handling when parsing/writing the config file, I just had a closer look at the Python 2.7 implementation of ConfigParser, especially its read() and write() methods. Judging from the code, I would say the read() method itself raises only exceptions based on ConfigParser.Error. However, the ConfigParser read() and write() methods internally use readline() and write() on File-like objects, which may throw IOError. Hence, I would suggest to catch ConfigParser.Error and IOError. All other Exceptions that could theoretically appear during read() or write() (e.g. KeyboardInterrupt or some runtime error) I would like to catch at a somehow more global point (maybe in Gui()) as they are not really related to the parsing the config file. What do you think?

[jensrudolf]

I totally agree with your last point, that this merge request is quite big. As you suggest, I will break this up into several smaller requests. I will try to split this up into pieces during the coming days as I find some time. Therefore, I close this one in favor of the split requests. Thanks for your comments.

[noseka1]

Thanks for investigating further on the exception topic. I wish the ConfigParser library would specify and limit which exact exceptions can be thrown by its APIs. I understand that you can review the ConfigParser's source code and draw conclusions about the current set of exceptions that can be thrown but what about the future ConfigParser implementations? In the future, ConfigParser can change the implementation and throw additional exceptions. Where I am getting at is that unless there is a clear API contract provided by the ConfigParser library I would prefer the defensive LinuxBand code that won't break in the future because of the possible changes in ConfigParser.

Handling exceptions at a more global point might be technically difficult as the LinuxBand code is pretty much a set of handlers called back by the UI library.

Thanks a lot for breaking up the pull request in smaller pieces. That's great. In the meantime I am working on porting LinuxBand from GTK2 to GTK3. I think that this needs to happen before migrating to Python 3.