Skip to content

Conversation

@ozhol
Copy link

@ozhol ozhol commented Apr 19, 2025

No description provided.

@missinglink
Copy link
Owner

Hi Alex, I don't really understand what the purpose of this change is and what functionality it improves.

Could you please write a description about why this change is needed?

@missinglink
Copy link
Owner

Hey Alex, copying from Linkedin for visibility:

I need to preserve two languages, but the current implementation only extracts one language from the PBF file. As a result, the streets layer only includes data in a single language.

The existing implementation only exports the "name" field, so I can see how that's limiting, although I don't love the pipe concatenation method you've used to expose the additional language(s): (ie. name = name + " | " + nameEn)

I see a few issues with this:

  • It's not backwards compatible
  • It's not extensible to other languages and tags
  • It requires the consumer to parse the string, (ie. splitting the text on the | char)
  • The Tags key named name no longer represents a tag in the source data verbatim (ie. it's the concatenation of two tags).

The existing implementation has a code comment which helps me remember what I was thinking here remove all tags except for 'name' to conserve storage space.

So in this case it sounds like you'd like to use a bit more storage in order to have additional tags available.

I'd suggest that instead of concatenating the string, instead store both in the Tags which get persisted to the database (ie. item.Tags = map[string]string{"name": name, "name:en": nameEn}.

Regarding extensibility (ie. the ability to add additional languages and tags), this can either be configurable so that it can be merged upstream into this repo, or can remain on a fork.

The final piece of the puzzle would then be to update any commands which use this handler (ie. command/street_merge.go) such that they output all the additional tags, for street_merge specifically we'd also need to look at the name matching logic to see how that would be affected and if it could be improved.

@missinglink
Copy link
Owner

I would be hesitant to merge a change which wasn't backwards compatible as I know there are already consumers using this code to generate extracts of OSM containing all the street intersections, changing the output format could break those implementations.

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