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

Don't print missing key errors when fallback is performed #2781

Closed
Tracked by #2259
sffc opened this issue Oct 24, 2022 · 3 comments · Fixed by #3262
Closed
Tracked by #2259

Don't print missing key errors when fallback is performed #2781

sffc opened this issue Oct 24, 2022 · 3 comments · Fixed by #3262
Assignees
Labels
C-data-infra Component: provider, datagen, fallback, adapters C-segmentation Component: Segmentation S-small Size: One afternoon (small bug fix or enhancement) T-techdebt Type: ICU4X code health and tech debt

Comments

@sffc
Copy link
Member

sffc commented Oct 24, 2022

Currently, a number of ICU4X examples print warnings such as

2022-10-24 14:21:05,150 WARN [icu_provider::error] ICU4X data error: Missing data for locale (key: segmenter/dictionary@1, request: km)

These are harmless warnings that occur as part of vertical fallback. We're going to keep getting issues about this (#2780) so we should silence these warnings if they occur during vertical fallback.

A way to fix this is for vertical fallback to set a value in the DataRequestMetadata and then to not output the missing key error if the metadata has the flag set.

@sffc sffc added C-data-infra Component: provider, datagen, fallback, adapters T-techdebt Type: ICU4X code health and tech debt S-small Size: One afternoon (small bug fix or enhancement) labels Oct 24, 2022
@sffc
Copy link
Member Author

sffc commented Oct 24, 2022

Note that in some cases, these warnings indicate data missing from testdata. For example, if the Khmer dictionaries are missing, maybe we should go ahead and print the warning. We should also verify that Segmenter doesn't try to load dictionary data if LSTM data is present.

@sffc sffc added the C-segmentation Component: Segmentation label Dec 1, 2022
@sffc
Copy link
Member Author

sffc commented Dec 1, 2022

We should discuss with @makotokato and @aethanyc and then triage.

@sffc sffc added the discuss Discuss at a future ICU4X-SC meeting label Dec 22, 2022
@sffc
Copy link
Member Author

sffc commented Dec 22, 2022

Maybe we want to have an annotation in the DataRequestMetadata for whether or not to print the warning. Vertical fallback can set the annotation, as well as Segmenter.

@sffc sffc self-assigned this Dec 22, 2022
@sffc sffc added this to the ICU4X 1.1 milestone Dec 22, 2022
@sffc sffc removed the discuss Discuss at a future ICU4X-SC meeting label Dec 22, 2022
@sffc sffc modified the milestones: ICU4X 1.1, ICU4X 1.2 Dec 22, 2022
@sffc sffc mentioned this issue Jan 11, 2023
22 tasks
@robertbastian robertbastian assigned robertbastian and unassigned sffc Apr 5, 2023
@sffc sffc closed this as completed Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-data-infra Component: provider, datagen, fallback, adapters C-segmentation Component: Segmentation S-small Size: One afternoon (small bug fix or enhancement) T-techdebt Type: ICU4X code health and tech debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants