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

road segment infra - improvements #2567

Closed
atalyaalon opened this issue Feb 14, 2024 · 6 comments
Closed

road segment infra - improvements #2567

atalyaalon opened this issue Feb 14, 2024 · 6 comments
Assignees

Comments

@atalyaalon
Copy link
Collaborator

atalyaalon commented Feb 14, 2024

Ziv, awesome work with the road segments infra (that includes junctions of course)

Following both this pr and this one

I think we should:

  1. Make sure we stop using road_segment_name in API at all, only road_segment_id (Also in news flashes - make sure we add to the news flash the road_segment_id rather than road_segment_name like we do today).
  2. Verify we have both road_segment_id and road1 in each interurban query (perhaps this already exists?), and perhaps raise error if not.
  3. Create a unified infra for location filters (per resolution type). I think we're almost there.
    Is it possible, for example, that we're missing a certain widget that does not use "get_expression_for_road_segment_location_fields" in one of the queries and hence we don't use the junctions data?
    (@ziv17 I know you've made a very thorough work - adding it to get_query function for example - but wondering how can we make sure we don't forget it in the next widget we're creating (it can also be by having a documentation on how to construct a query and not by code)
    Of course we need to consider different queries that are a part of the code but do not filter the segment for some reason (for example, in head_on_collisions_comparison_widget.py we have a query of all roads in Israel - hence we don't filter the road segment there)
@atalyaalon atalyaalon changed the title road segment data infra road segment infra - improvements Feb 14, 2024
@ziv17
Copy link
Collaborator

ziv17 commented Feb 24, 2024

Hi @atalyaalon ,
regarding No2 above, "Verify we have both road_segment_id and road1". Road segment identifies also the road. Why do we require to get also the road?

@ziv17
Copy link
Collaborator

ziv17 commented Feb 28, 2024

Hi @atalyaalon
by "Make sure we stop using road_segment_name in API at all", you mean to fail requests that use it, or convert it to road_segment_id?

@atalyaalon
Copy link
Collaborator Author

atalyaalon commented Mar 13, 2024

Hi @atalyaalon , regarding No2 above, "Verify we have both road_segment_id and road1". Road segment identifies also the road. Why do we require to get also the road?

as discussed we don't need it to have both in the query. What we might need is to make sure road1 is in "location_info" and in "news_flash" object, since the road1 (and also road_segment_name) is used in titles for example

@atalyaalon
Copy link
Collaborator Author

atalyaalon commented Mar 13, 2024

Reference in new

as discussed, yes, we don't want to query by road_segment_name, only by id

@ziv17
Copy link
Collaborator

ziv17 commented May 5, 2024

Hi @atalyaalon ,
I think the location accuracy PR (#2641) closes this issue.

@ziv17
Copy link
Collaborator

ziv17 commented May 24, 2024

PR (#2641) closed this issue.

@ziv17 ziv17 closed this as completed May 24, 2024
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