Python3 migration #160
Python3 migration #160
Conversation
not sure this is actually the best fix for that 🤔
ResourceWarning: unclosed file (x3)
|
Wow, @nlehuby, this is awesome 🎉 |
|
@nlehuby 👏 👏 I tried the PR and it works great, just requires a little change in two line to add binary mode on read/write operations. Also I made some improvements related with installing the forked transitfeed module from git. I'll made PR with this changes to @Jungle-Bus repo for her consideration. |
* Install transitfeed fork from git * Update install command and requirements * Add binary mode on I/O operations * Install on Travis with the setup file and use python3 * Delete depedency_links from setup file thanks @jamescr
|
Thanks for these improvements @jamescr , I've updated the PR 👍 Does anyone want to have a look ? |
|
I've made a quick review, and I don't have any comment 👍 ( but I don't feel relevant enough to approve ) |
| @@ -10,7 +10,7 @@ | |||
| keywords='openstreetmap gtfs schedule public-transportation python', | |||
| author='Various collaborators: https://github.com/grote/osm2gtfs/graphs/contributors', | |||
|
|
|||
| install_requires=['attrs>=17.1.0', 'overpy>=0.4', 'transitfeed>=1.2.16', 'mock', 'webcolors', 'transporthours'], | |||
| install_requires=['attrs>=17.1.0', 'overpy>=0.4', 'transitfeed @ git+https://github.com/pecalleja/transitfeed.git@python3', 'mock', 'webcolors', 'transporthours'], | |||
There was a problem hiding this comment.
As https://github.com/google/transitfeed is no longer actively maintained, it's probably okay to use such a fork. Wondering what are the alternatives on the long run, because the recommended alternative https://github.com/MobilityData/gtfs-validator only does validation, not parsing for Python, right?
|
I guess it would be nice if this can be reviewed / merged :-) @xamanu ? |
|
Thanks for the input, @amenk. Unfortunately, I am totally out of the context. Feel free to proceed without me. |
|
Looks good basically :-) But: No clue if that is related to the Python3 port, but we are using this in a GitHub action which autocommits the GTFS and the fields in the .txt files change their order seemingly randomly on each run. I did not notice this when we were still on Python 2 Example: AddisMap/AddisMapTransit-gtfs@f8666c5 Can have a totally unrelated reason or maybe it is even like this before the upgrade, but we did not notice it before. Just wanted to mention it. Also it's a very niche issue so it still could be merged :-) |
|
Can this be Merged? |
Here is a new version of osm2gtfs for python3 🎉
The most significant changes are:
dict.items()instead ofdict.iteritems()urllib2tourllib.requestI've also fixed some python warnings on memory issues about unclosed files and split the tests into several travis jobs so it can be easy to see which one is failing. And they are all green ✔️ ✔️
As the
transitfeedlib has not been released yet in python3, I'm using a fork (that needs to be cloned and installed before installing osm2gtfs).I did not cherry-picked the changes of @xamanu from #153 about the formatting to keep this PR as reviewable as possible (but I do think we should add them as soon as we have a working version of osm2gtfs for python3 on master)