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

Add oneline templates #103

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

Zverik
Copy link
Contributor

@Zverik Zverik commented Feb 2, 2024

So this is a bit hard to describe, but the goal for making this new file was simple: to have a good one-line address built from parts.

I tried using the original template library, but it doesn't help in multiple ways:

  • Sometimes the order is reversed: country, city, house number, so taking the first line doesn't work.
  • And no, looking for a road name on all lines doesn't work either, since it can be missing.
  • Postcodes almost never belong to a one-line address description, except maybe in the UK. But they are too prominent in templates.
  • To have multiple options for a line (e.g. short and long), the address should be built from parts. Alas the templates produce a single multiline string.

So I have made this new file, by hand. It doesn't need a Mustache parser, and it's more structured. Basically you get the most likely value for 5 parts: local house name, street address, city part, city name, and region. From that, you can, for example, take the first non-empty value for a short address, or make up an algorithm to build a longer line.

Also having country the last part of the region keys makes it easier to deal with disputed territories, since the country never comes up there, shadowed by a region name or something else.

Topics to discuss:

  • Is this even needed in this repository?
  • Is oneline.yaml a proper name for the file? Maybe something like structured.yaml, or put it in a different place?
  • How should it be documented? It uses a subset of original keys, without fallbacks or post-format replacements.
  • Is the validation script in Python fine? It depends only on pyyaml library. I've included a CI script.

I am fine with dropping this pull request if it doesn't match the repo's purpose. Just wanted to share the thing I spent some days on for my job :)

@freyfogle
Copy link
Member

hmm, let me have a longer think and look but my very initial reaction is

  • this is a different thing
  • seems a real shame if we can not keep one source of truth for the formats
  • not really excited about adding a dependency on yet another programming language

will have a longer look early next week

@Zverik
Copy link
Contributor Author

Zverik commented Feb 2, 2024

Okay I've become a bit rusty in perl, but rewrote the validation into it. That covers your last item.

Regarding the first one: this is address formatting. Just a different one. For starters, I needed to print "road, house number" or "house number, road" according to what's common to the country. That's why I got to this repository in the first place.

Turns out it works, but only when you need a full address printed. If you need a part (e.g. a street address only), you have to either do a complex string processing, or... I don't know, do the same thing I did.

Each country has a different set of fields and their priorities for each value, and a different order for addresses. Some reference city_district, some don't have city parts at all, some mention state in regions, and some use island instead. It was all in the original templates, just unstructured.

With the original templates, you got one option: print a full address. Which works if you're targeting the entire world. If you are printing an address line for a local, who knows which country and state they are in, you might want to trim the line. You cannot do that with the original templates, but can — with the structured lists in the new file.

@freyfogle
Copy link
Member

yes, the use case makes sense and is clear, and you are right, the current set-up is not good for that.

Still I wonder is there a way not to duplicate much of the logic of worldwide.yaml in oneline.yaml, otherwise we will get out of sync (your change to the entry for DO being a great example).

Thanks for switching to perl!

@Zverik
Copy link
Contributor Author

Zverik commented Feb 7, 2024

What do you think of merging the new file into the old one? E.g.

# Switzerland
CH:
    address_template: |
        {{{attention}}}
        {{{house}}}
        {{{road}}} {{{house_number}}}
        {{{postcode}}} {{#first}} {{{postal_city}}} || {{{town}}} || {{{city}}} || {{{municipality}}} || {{{village}}} || {{{hamlet}}} || {{{county}}} || {{{state}}} {{/first}}
        {{{country}}}
    parts:
        - [house]
        - [road, house_number]
        - []
        - [postal_city, town, city, municipality, village, hamlet, county, state]
        - [country]
    replace:
        - ["Verwaltungskreis",""]
        - ["Verwaltungsregion",""]
        - [" administrative district",""]
        - [" administrative region",""]

I think virtually all replace parts were preseved, and postformat_replace are not used for parts.

(Also I've been wondering if I should keep [attention, house] instead of just [house], albeit the meaning is somewhat different.)

@freyfogle
Copy link
Member

Hi Ilya,

sorry for the delay, very busy week.

That would be a good solution to keep things all in one place which is definitely an improvement over having multiple files. I guess my only slight hesitation is having two different styles of templating. What I like about mustache (besides it being a well-known commonly used format with parsers in most/all programming languages) is the logic is clear in the template. In your case the logic is in your processing code, which makes reuse across languages more difficult.

re: [attention, house] I guess it depends on your use case. In general my thinking was to keep the templates comprehensive and if you don't want a specific value then make sure it is not set when calling the formatter.

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.

None yet

2 participants