Making PolarRoute-server compatible with latest PolarRoute and MeshiPhi versions - #175
Making PolarRoute-server compatible with latest PolarRoute and MeshiPhi versions#175thomaszwagerman wants to merge 10 commits into
Conversation
|
I appreciate this is still in draft, but looks good so far, I'm very pleased to finally be getting rid of the need to deal with tempfiles now we can use the python interface more directly. Feel free to make any other improvements to this function you feel are necessary. I'm sure you will, but don't forget to restrict the polar-route version in the pyproject.toml from this point. |
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Satisfied this successfully runs and returns routes |
davidwyld
left a comment
There was a problem hiding this comment.
As I've said, this is a huge improvement, thanks for doing this.
I wonder if we should add to the testing of this functionality in test_utils? I'm happy to write a test if you don't have time.
Don't forget to update the changelog in this PR too.
Co-authored-by: David Wyld <24752124+davidwyld@users.noreply.github.com>
|
I've added Opened an issue in PolarRoute for related issues within |
davidwyld
left a comment
There was a problem hiding this comment.
Worth discussing whether we want to expose route_type in the API?
Want me to implement a test for evaluate_route? Or we can just open an issue since there isn't one to begin with so I don't want it to hold back this branch.
| df = pd.DataFrame(df_data) | ||
|
|
||
| # Get start and end waypoint names | ||
| from_wp = "waypoint_0" |
There was a problem hiding this comment.
I'm not sure if it's possible for the submitted route to have waypoint names, we may want to preserve those in the future, but lets leave this as is for now, they don't really matter for SIIS at least.
There was a problem hiding this comment.
TZ review geojson schema and safety of waypoint name here
There was a problem hiding this comment.
It would be much better IMO if route_calc took a geojson/dict as input rather than a dataframe, but whatever, this is fine.
It would be nice to preserve waypoint names from the geojson if it has them
There was a problem hiding this comment.
waypoint names preserved with e6a2b24, agree it would be lot less hassle if route_calc took a GeoJSON though - although that is a PolarRoute change. Might investigate.
There was a problem hiding this comment.
see bas-logist/PolarRoute#355 - I think worth holding off on this until that is reviewed, as well as possibly implementing bas-logist/PolarRoute#350 as it touches this
Specifically updating how
evaluate_routeworks with PolarRoute changes toroute_calcapi:Also removing all
TempFilerelated code, as now longer required for new API.