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

Few improvements to Logistics example #14

Open
nilsnolde opened this issue May 28, 2018 · 2 comments
Open

Few improvements to Logistics example #14

nilsnolde opened this issue May 28, 2018 · 2 comments
Assignees

Comments

@nilsnolde
Copy link
Contributor

  1. No need for HD polygon here actually. You're not using it except for plotting. Think you can leave it out.
  2. The first locations list:
  • Only give the addresses and demands as assumption and use geocoding (the Pelias endpoint pls) to get the coordinates
  • change the order to address, demand, long, lat
  1. I generally like the approach with numpy and pandas, but try to keep your variables named, i.e. try to avoid to do e.g. route[:, ::-1].T. It's best done in dictionaries, I find, where you can put named keys. It makes it a lot nicer and easier to read.
  2. reversed for an iterator is preferrable over [::-1]. Both for performance and readability.

Apart from that, very well done. I know, it can be hard, but try to think most practical with these examples, a fluid storytelling with just enough info to solve a certain problem in the most elegant and readable way.

@thegialeo
Copy link
Contributor

Ok, i will work on those points this week, presumably on friday. Thanks for the feedback.

@thegialeo
Copy link
Contributor

thegialeo commented Jun 2, 2018

  1. i removed the HD polygon

  2. done

  3. The problem is that the direction API return a 2d list: route = np.array(response['routes'][0]['geometry']), however PolyLine() requires a 1d list of tuples. the expression "list(zip(route[:, ::-1].T[0], route[:, ::-1].T[1]))" covert the 2d list reponse into a 1d list of tuples. numpy is the only way i know to do this in one line, if there is a better way of doing it, i am excited to hear. I now create an extra variable for it and added a comment on what it does to improve readablity (please look at the notebook and tell me if it fullfils what you wanted)
    I change routes from a 2d list to a dictionary of 1d lists. This only change the line points = locations[route] to points = locations[routes[route]]. Is this what you meant with "best done in dictionaries"?

  4. i am not sure what to do here. Is their a specific code line, where you wish to have reserved() instead of [::-1], because I never iterate over a list backwards (with [::-1] or otherwise) in the notebook

If there are any other improvement that can be made, feel free to tell me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants