Python map import scripts refactor

[Conengmo]

Hi guys,

I'm working on figuring out how Barefoot really works underneath. While I was looking through the Python scripts that import map data, I figured they could use some work. I wanted to show my work in progress to maybe get some comments if and how this could be merged in main stream Barefoot when finished.

What I'm working on:

• Mainly making it DRY
• Making it Python 3 compatible
• Improve readability
• Keep functionality the same

I know this will be hard to review because the changes are so large. But know that I'm not changing functionality in this PR.

[Conengmo]

I touched all the Python code and finished a first refactoring. It can be better, but it is functional now.

[smattheis]

Thanks for refactoring and the PR!

@jongiddy Would you mind having a look on the changes? You're much better in Python than me. I have looked through it, e.g., if conditions, test execution, etc., and it seems all okay (not bypassing any test) and the CI system checked it with perfect results, i.e., also the result of the sample data looks the same as before.

@Conengmo Have you used some code style configuration? If so, could you include that in the repo in order to have the same style with future edits? Further, is the PR still WIP?

[Conengmo]

Good to hear you want to consider this PR. I follow regular PEP8, the default style guide for Python:

https://www.python.org/dev/peps/pep-0008/

Is there a line width you want to stick to in this project? I tried to keep most at 80 characters, but allowed for lines to go to 100 characters. Personally I like 100.

If you want to consider merging this, I'll take this weekend to polish it a bit more before dropping the WIP. Let me know if you have any comments or requests!

[smattheis]

In the Java code style, we use a line width of 100 characters. PEP8 is perfect and can be integrated into the Travis CI configuration. Thanks!

[Conengmo]

Does Travis also do code style reviews? Another project I work on uses Travis for testing and Stickler for automated code style review:

https://stickler-ci.com/

[Conengmo]

Alright I finished cleaning up. The Travis test passed, so that's nice. I hope you guys like it, let me know if you have any further requests or suggestions.

[jongiddy]

This is a very nice clean-up of the Python code. I have tested the new code by creating a database from the same OSM file using master and this branch. They appear to create the same database.

The Python code could definitely do with more automated tests so we are more comfortable with such major refactorings.

This change will also make it easier to add a pep8 test to the CI, now that the code is fairly PEP8 clean (with an adjustment to max line size of 100).

Thanks for doing this. It has been on my mind as a TODO for some time.

[Conengmo]

Glad you like it. I agree on adding more tests, and also a good idea to add a pep8 check to Travis. But that's something for a new PR I reckon. I'll leave this PR as is so it won't have to be reviewed again.

[thomhubers]

So since it's approved and everyone's happy. Can this be merged?