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 CLDR data source for subdivion translations. #473

Merged
merged 4 commits into from
Aug 17, 2017

Conversation

rposborne
Copy link
Collaborator

@rposborne rposborne commented Aug 15, 2017

This massive improves the number of translations we have available and lays the groundwork of other importers.

Thanks to @coderifous and @mig for the help on this.

This fixes #437

@jawnsy
Copy link

jawnsy commented Aug 15, 2017

I perused these changes quickly and they look good to me. I think it's a better approach than what I proposed with #438 because it handles updates better (regrettably I didn't understand the way updates are currently handled and couldn't do as good a job as you did there).

Unfortunately I can't test the changes as I left Red Hat and no longer have access to the source code of the project I was working on.

One thing I noticed is that many fields where the data is presumably unknown are recorded as empty fields, and I wonder if it would be better to exclude them from the file entirely? For example, in AM.yaml:

  geo:
    latitude: 
    longitude: 
    min_latitude: 
    min_longitude: 
    max_latitude: 
    max_longitude: 
  name: Gegark'unik'

It seems the whole geo section can be excluded? Over all of the input files this might save some disk space, though probably not a huge amount, and it doesn't matter much with compression.

@rposborne
Copy link
Collaborator Author

@jawnsy it looks like geo is filled out for most sub divisions. You make an interesting case though to remove fields that have nil values. Or pass the cache through a compression layer, which currently it is not. I would be curious to see what gives us a better performance gain.

I think for now this is focused on data integrity and data import over the consumption of the data. So I'll break those out into separate issues.

@jawnsy
Copy link

jawnsy commented Aug 17, 2017

@rposborne Sounds like a reasonable approach to me! Thanks for your work!

@rposborne rposborne merged commit ea53b6b into countries:master Aug 17, 2017
@rposborne rposborne deleted the cldr-subdivisions branch December 26, 2017 23:12
@rposborne
Copy link
Collaborator Author

@jawnsy 2.1.3 released which includes this change

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.

Some countries are missing subdivisions
2 participants