Skip to content

Hardcode tag_conditions#2

Merged
Robbendebiene merged 62 commits intoOPENER-next:mainfrom
wielandb:main
Sep 27, 2023
Merged

Hardcode tag_conditions#2
Robbendebiene merged 62 commits intoOPENER-next:mainfrom
wielandb:main

Conversation

@wielandb
Copy link
Copy Markdown
Contributor

No description provided.

@7h30n3 7h30n3 marked this pull request as ready for review August 18, 2023 12:05
Copy link
Copy Markdown
Member

@Robbendebiene Robbendebiene left a comment

Choose a reason for hiding this comment

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

Please sort everything by DELFI ID (this is mostly the case but not consistent)

And also run a JSON formatter over it.

Comment on lines +194 to +221
{
"DELFI_ID": 1073,
"LABEL": "für Rollstuhlfahrer zugängliche Toilette ist mit Euroschlüssel nutzbar",
"CONDITION": {
"osm_tags": {
"amenity": "toilets",
"access": ["yes", "customers"]
},
"osm_element": ["Node", "ClosedWay"]
},
"TAGS": {
"centralkey": "eurokey"
}
},
{
"DELFI_ID": 1074,
"LABEL": "für Rollstuhlfahrer zugängliche Toilette ist mit einem speziellen Schlüssel nutzbar, der beim örtlichen Einzelhandel zu den Öffnungszeiten ausgeliehen werden kann",
"CONDITION": {
"osm_tags": {
"amenity": "toilets",
"access": ["yes", "customers"]
},
"osm_element": ["Node", "ClosedWay"]
},
"TAGS": {
"locked:description": []
}
},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needs to be reworked/discussed.

Copy link
Copy Markdown
Member

@Robbendebiene Robbendebiene left a comment

Choose a reason for hiding this comment

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

Please also add 2080 based on our question catalog and the DELFI Handbuch.

Comment on lines +421 to +434
"DELFI_ID": 1131,
"LABEL": "Mit akustischer Ausgabe",
"CONDITION": {
"osm_tags": {
"departures_board": "realtime"
}
},
"TAGS": {
"departures_board:speech_output": [
"yes",
"no"
]
}
},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is tricky. I think we already discussed about the problem that we cannot really detect if a "departures_board" is mapped as a separate element. However since 1130 (tries) to checks for this 1131 should reflect this and only check for speech_output=yes/no. Or we go the other way around and adjust 1130 (so it matches on station elements and checks for "departures_board": "realtime") which was probably the intention of our last discussion.

My personal preference would be to map station wide departure boards as separate elements, but I leave the decision to you.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Discussion] Do both, split it.

"osm_element": "OpenWay"
},
"TAGS": {
"incline": []
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know that the wiki states this, but looking at the DELFI catalogue what do you think is correct here?

I feel like

  • 2132 is conveying=forward/backward
  • 2133 is conveying=reversible

Because an escalator can have incline=up or any positive value while still only being usable "downwards". In other words conveying=yes and incline=* doesn't tell me the direction the escalator is moving.

If you agree with me then we should also adjust the wiki.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Discussion] Valid values for 2132 and 2133 are forward/backward/reversible

@Robbendebiene Robbendebiene merged commit 2820fcf into OPENER-next:main Sep 27, 2023
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.

2 participants