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

Export data from all number systems #189

Merged
merged 2 commits into from
Oct 18, 2022
Merged

Conversation

movermeyer
Copy link
Collaborator

@movermeyer movermeyer commented Aug 15, 2022

What are you trying to accomplish?

Fixes #89
Fixes #167

ruby-cldr was outputting number data from arbitrary number systems based on whatever data it first encountered. This resulted in the data from multiple number systems getting incorrectly mixed in the output.

What approach did you choose and why?

This is a large refactor/overhaul of the existing Numbers component.
It adds the number system as a new level of nesting in the output, and changes the key structure to more closely match the structure and terminology of the upstream CLDR data.

The Numbers component now outputs the number data for all number systems.

What should reviewers focus on?

...

The impact of these changes

This is a breaking change for downstream consumers, since the key structure of the output numbers.yml files has changed significantly with this PR:

  • The useless number_system key has been removed; the keys are now nested under the number system
  • The keys include the standard subtype (meaning that other subtypes are now able to be exported. e.g., accounting)
Before: numbers.formats.decimal.patterns.short.1000000000
After:  numbers.latn.formats.decimal.patterns.short.standard.1000000000

Testing

...

Checklist

  • I have added a CHANGELOG entry for this change (or determined that it isn't needed)

@movermeyer movermeyer force-pushed the movermeyer/number_systems branch from 91a58e0 to 51a1b5d Compare August 17, 2022 13:14
@movermeyer movermeyer marked this pull request as ready for review August 17, 2022 15:08
@movermeyer movermeyer force-pushed the movermeyer/number_systems branch from 51a1b5d to beeb7f4 Compare August 17, 2022 15:09
Copy link
Collaborator

@trishrempel trishrempel left a comment

Choose a reason for hiding this comment

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

Looks good; I have some minor suggestions and questions.

lib/cldr/export/data/numbers.rb Outdated Show resolved Hide resolved
lib/cldr/export/data/numbers.rb Outdated Show resolved Hide resolved
lib/cldr/export/data/numbers.rb Outdated Show resolved Hide resolved
lib/cldr/export/data/numbers.rb Outdated Show resolved Hide resolved
lib/cldr/export/data/numbers.rb Outdated Show resolved Hide resolved
lib/cldr/export/data/numbers.rb Outdated Show resolved Hide resolved
lib/cldr/export/data/numbers.rb Outdated Show resolved Hide resolved
@movermeyer movermeyer force-pushed the movermeyer/number_systems branch 3 times, most recently from 6daddb5 to 39a8945 Compare September 9, 2022 17:46
@movermeyer movermeyer force-pushed the movermeyer/number_systems branch from 39a8945 to a571cf8 Compare October 18, 2022 18:50
@movermeyer movermeyer force-pushed the movermeyer/number_systems branch from a571cf8 to 2c85975 Compare October 18, 2022 19:16
@movermeyer movermeyer merged commit 701c267 into main Oct 18, 2022
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.

Export data from different numbering systems Don't mix results from different number systems
2 participants