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

[Jormun] optim geovelo #2711

Merged
merged 6 commits into from
Mar 20, 2019
Merged

[Jormun] optim geovelo #2711

merged 6 commits into from
Mar 20, 2019

Conversation

xlqian
Copy link
Member

@xlqian xlqian commented Mar 14, 2019

https://jira.kisio.org/browse/NAVITIAII-2675

-> use compress protocol
-> simplify useless fields
-> use ujson
-> pythonic optim

image

@xlqian xlqian changed the title optim geovelo [Jormun] optim geovelo Mar 14, 2019
routing.routing_status = response_pb2.unknown

add_ = row.routing_response.add
for e in itertools.islice(json_response, 1, sys.maxint):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use None for stop?
And are we sure that geovelo keep the request order? And by sure I mean does is it documented?

Copy link
Member Author

Choose a reason for hiding this comment

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

since in the old code, we didn't check the order... I assumed that the order was kept

)
self._check_response(r)
resp_json = r.json()
resp_json = ujson.loads(r.text)
Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/kennethreitz/requests/issues/1595
It will be nice to have it for everyone :)

Copy link
Member Author

@xlqian xlqian Mar 14, 2019

Choose a reason for hiding this comment

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

actually, i was looking forward to being able to specify the json module in .json(), like r.json(json_module=ujson)...

Copy link
Member Author

Choose a reason for hiding this comment

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

mypy doesn't like requests.models.json = ujson

@@ -104,8 +108,8 @@ def _make_request_arguments_bike_details(cls, bike_speed_mps):
@classmethod
def _make_request_arguments_isochrone(cls, origins, destinations, bike_speed_mps=3.33):
return {
'starts': [cls._pt_object_summary_isochrone(o) for o in origins],
'ends': [cls._pt_object_summary_isochrone(o) for o in destinations],
'starts': (cls._pt_object_summary_isochrone(o) for o in origins),
Copy link
Member Author

Choose a reason for hiding this comment

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

usjon has difficulties with generators as reported in several issues... what a pity...

@codecov-io
Copy link

codecov-io commented Mar 15, 2019

Codecov Report

Merging #2711 into dev will decrease coverage by <.1%.
The diff coverage is 100%.

@@           Coverage Diff           @@
##             dev   #2711     +/-   ##
=======================================
- Coverage   86.9%   86.8%   -0.1%     
=======================================
  Files        392     392             
  Lines      58877   58842     -35     
=======================================
- Hits       51174   51122     -52     
- Misses      7703    7720     +17

@xlqian xlqian requested a review from kinnou02 March 18, 2019 13:05
@xlqian xlqian merged commit beeaba5 into hove-io:dev Mar 20, 2019
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

Successfully merging this pull request may close these issues.

None yet

6 participants