[Jormungandr] Manage address within zones in journeys #4071
Conversation
|
Please retry analysis of this Pull-Request directly on SonarCloud. |
|
Please retry analysis of this Pull-Request directly on SonarCloud. |
| if MAP_STRING_PTOBJECT_TYPE.get(dict_pt_object.get("embedded_type")) != type_pb2.ADDRESS: | ||
| return dict_pt_object |
There was a problem hiding this comment.
I'm wondering if you should have a second sub-function here. The goal of a sub-function is for documentation. Since the sub-function also has a name, it helps to understand a sub-part of the code.
| if MAP_STRING_PTOBJECT_TYPE.get(dict_pt_object.get("embedded_type")) != type_pb2.ADDRESS: | |
| return dict_pt_object | |
| if MAP_STRING_PTOBJECT_TYPE.get(dict_pt_object.get("embedded_type")) == type_pb2.ADDRESS: | |
| replace_address_with_custom_poi(dict_pt_object, uri) | |
| else: | |
| return dict_pt_object |
All the code below would go in the new function.
| return None | ||
| if MAP_STRING_PTOBJECT_TYPE.get(dict_pt_object.get("embedded_type")) != type_pb2.ADDRESS: | ||
| return dict_pt_object | ||
| first_within_zone = next( |
There was a problem hiding this comment.
You could rename this new_poi to express better the fact that we're actually creating a new object (based on an existing POI, but with custom coordinates).
| first_within_zone["poi"]["coord"]["lon"] = "{}".format(lon) | ||
| first_within_zone["poi"]["coord"]["lat"] = "{}".format(lat) |
There was a problem hiding this comment.
I would put a comment here to express why we're changing the coordinate. Here is a suggestion.
# We're putting back the coordinate of the end-user, who is in a POI area (with entrypoints).
# If we don't, barycenter of the POI area will be displayed which means nothing for the end user.
| @@ -494,6 +494,30 @@ def get_pt_object_from_json(dict_pt_object, instance): | |||
| return pt_object | |||
|
|
|||
|
|
|||
| def transform_entrypoint(dict_pt_object, uri): | |||
There was a problem hiding this comment.
I can't find a good name for this function... but I'd say we should try to find another name. Here are suggestions for brainstorming, maybe you will find other ideas?
Building the name of the function with this shape.
<verb>_<name>
<verbs>:
- improve
- customize
- replace
- rectify
<names>:
- entrypoint_detail
- address (
replace_addressfor example)
other ideas:
rectify_entrypoint_with_urientrypoint_uri_refocus(I kind of like this one, we're changing back the entrypoint to refocus on the end-user information: thefromor theto)
None of them are really good, but I'm hoping to help find ideas.
| @@ -494,6 +494,36 @@ def get_pt_object_from_json(dict_pt_object, instance): | |||
| return pt_object | |||
|
|
|||
|
|
|||
| def replace_address_with_custom_poi(dict_pt_object, uri): | |||
| new_poi = next( | |||
There was a problem hiding this comment.
my question may be pedantic:
could the address be included in more than one shape? if so, how could you be sure that the within_zones are sorted in a deterministic order?
There was a problem hiding this comment.
On bragi, the within_zones are order by distance to the coordinate. See https://github.com/hove-io/mimirsbrunn/pull/849/files.
|
Please retry analysis of this Pull-Request directly on SonarCloud. |
pbougue
changed the title
Jira: NAV-2158
From = coord in Poi shape (Jardin du Luxembourg (Paris))
From=POI (Jardin du Luxembourg (Paris))
