Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/osmnx import #64

Merged
merged 9 commits into from
Mar 10, 2021
Merged

Fix/osmnx import #64

merged 9 commits into from
Mar 10, 2021

Conversation

joroeder
Copy link
Member

@joroeder joroeder commented Nov 16, 2020

The osmnx_import example is not running.

On thing is that there was a wrong attribute name distance instead dist.

The other issue, I don't know, I commented the lines. The principal import works now, however, likely not properly at the current state.
Could you have a look at it @jnnr ?

@jnnr
Copy link
Member

jnnr commented Nov 16, 2020

I think it would be good to solve this before continuing the actual issue with the division of edges ..

So this does not fix issue #48? Then it should not say so. Can you instead describe the issue you are solving with this PR? Can be in the first comment of this PR, but would be very helpful!

@joroeder
Copy link
Member Author

joroeder commented Nov 16, 2020

So this does not fix issue #48?

This PR was indendet to do so, and it is not finshed yet ;-)

However, I see your point. It makes sense to use a separate PR for that. I will update the PR description.

@joroeder joroeder added the bug Something isn't working label Nov 16, 2020
@joroeder
Copy link
Member Author

the distance attribute changed in osmnx v0.13.0:

https://github.com/gboeing/osmnx/blob/master/CHANGELOG.md#0130-2020-05-25

Copy link
Member Author

@joroeder joroeder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some general questions and comments on the osmnx import

@joroeder
Copy link
Member Author

Some general questions and comments on the osmnx import

Please ignore this comment.

@joroeder
Copy link
Member Author

Okay, I fixed the issues regarding the not-running osmnx-import example, which means an update according to the actual osmnx version (0.16.1). I added this version requirement to the extras_require of setup.py.

The other issues (like #48) of the osmnx import are not solved with this PR.

@joroeder joroeder requested a review from jnnr November 19, 2020 12:52
@joroeder
Copy link
Member Author

@jnnr Since this branch is for review for more than 4 months now ;-) and it contains only small fixes by updating the API of the osmnx package, I would merge this branch if I don't hear anything by the end of next week.

Copy link
Member

@jnnr jnnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested successfully.

@joroeder joroeder mentioned this pull request Mar 10, 2021
5 tasks
@joroeder joroeder merged commit 2173bf3 into dev Mar 10, 2021
@joroeder joroeder deleted the Fix/osmnx_import branch March 10, 2021 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants