1647 pedestrian city vs street accident widget#1726
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #1726 +/- ##
==========================================
- Coverage 53.63% 52.07% -1.57%
==========================================
Files 56 56
Lines 6272 6316 +44
==========================================
- Hits 3364 3289 -75
- Misses 2908 3027 +119
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
BarVolunteering
left a comment
There was a problem hiding this comment.
The logic and the code are good, the only the thing that i am not sure about is the is_urban - I don't know if we have any other widgets that are urban, and how they are working(if they exist). - maybe someone else will know.
@assafdayan |
As at the time of implementation there was no rule based engine (is there now?) I wanted to refrain from checking location type in each widget as inter-urban and urban have mutually exclusive fields in the location. Once we have rules in place it can be removed. |
| is_urban = is_location_inner_city(request_params.location_info) | ||
| for w in get_widget_factories(): | ||
| widget: Widget = w(request_params) | ||
| if is_urban != widget.is_urban(): | ||
| continue |
There was a problem hiding this comment.
Urban is the first categorization of a newsflash by which a widget decides whether it is relevant. Soon we will have many additional categories. The different categories should not appear on the infrastructure level. I have two suggestions until the ranking will be operational:
1 - Simply return empty items, and generate_widgets() will not enter it to the results
2 - instead of is_urban(location_info), use is_included(request_params)
This wizard (as far as I can tell) is the first one implemented that requires an urban location (street rather than road and yishuv name).
I am assuming that the type of location affects which wizards are relevant. I added a boolean method to Wizard class is_urban and considered its relation to the newsflash location type in generate_widgets.
I then went ahead to implement the widget in accordance with its reddish specification and verified I got the same numbers.
closes #1647