Add support for direction tag into viewpoint pois#1916
Add support for direction tag into viewpoint pois#1916nvkelso merged 6 commits intotilezen:masterfrom
Conversation
rmarianski
left a comment
There was a problem hiding this comment.
Thanks! This seems fine to me, but deferring to @nvkelso if this is cool to add in.
Mind adding a quick test for this? You should be able to copy an existing one from the integration-test directory.
|
👀 |
|
This connects with #598 in the v1.9.0 milestone. |
nvkelso
left a comment
There was a problem hiding this comment.
Thanks for tackling this! Please update it as detailed below and we can get this merged ASAP :)
Because direction is sometimes not provided as a numerical degree (eg a value of 90 meaning 90°), like E for "east" = 90 and so on as detailed #598 (comment), we'll also need to add a new vectordatasource transform in Python to ensure values are in numerical range of 0-359 after also converting valid string input to numerical values. If the input isn't valid we should drop the property.
An existing transform to model off is vectordatasource.transform.height_to_meters or vectordatasource.transform.pois_capacity_int.
Note there's also a helper function to _to_float_meters that we'd also need to duplicate (and call something like _to_int_degrees (we could do float, but if so we'd only want to the tenths place?).
Once the transform Python code is added, we also need to trigger it by adding new line after:
So every feature in the POIs layer has the transform applied.
Then we'd need to add a test like Rob suggested... and make sure the test file tested for valid numerical, out of range numerical, and E > 90 conversions.
| output: | ||
| <<: *output_properties | ||
| kind: viewpoint | ||
| direction: {col: direction} |
There was a problem hiding this comment.
There are many more POIs that include direction, so let's move this to the very top of the YAML doc after wikidata_id: {col: wikidata} in the
global:
- &output_properties:
section, please.
|
@nvkelso I am sorry for this late answer, I was quite busy and it is also summer time :). I have tried to implement it as you wrote, but I am not sure if I understand it correctly, so if needed I can make further changes. Also could I ask you for advise how to write tests for this? |
|
This looks good :) I'll have a look this week and add the tests, thank you! |
|
Going to merge this and add tests in new PR. |
|
Fixed a few edge case bugs and added tests, docs in #1921. |
Add support for direction tag into viewpoint pois.
Connects with #598.