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

Replace language arg by standard translation mechanism #140

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

Bisaloo
Copy link
Member

@Bisaloo Bisaloo commented Jul 31, 2024

Fix #138

Todo:

  • Move the Spanish translations to the po/R-es.po file
  • Possible remove the translation arg from describe_ethnicity() as well
  • Compile to .po files into .mo with potools::po_compile()
  • Document how to get translated text in plots
  • Document the breaking change in NEWS

@juan-umana @Juanmontenegro99, this is a starting point to fix #138 using what we discussed yesterday in the development meeting (https://michaelchirico.github.io/potools/). Could you have a look and take this PR from here by finalizing the todos if you agree with this change.

It removes a large portion of duplicated code so it's an important improvement in terms of maintainability

@juan-umana
Copy link
Member

Thanks, @Bisaloo. The translation looks great and will be useful for Spanish-, Portuguese- and French-speaking countries in LAC also. We have some questions regarding the regional configuration of R that a user may have.

From what we understand, potools identifies the locale using Sys.getlocale() to choose the appropriate translation for the users. We also found that users can change their regional configuration using Sys.setlocale().

Then, when a user has its configuration in English (for expample) but wants to use epiCo (exclusively) in Spanish, without changing the configuration: should we ask the user to adjust their locale settings before using epiCo, or is it something we can handle within epiCo's functions?

@Bisaloo
Copy link
Member Author

Bisaloo commented Aug 26, 2024

Then, when a user has its configuration in English (for expample) but wants to use epiCo (exclusively) in Spanish, without changing the configuration: should we ask the user to adjust their locale settings before using epiCo, or is it something we can handle within epiCo's functions?

Not easily, because the expectation is that users would likely prefer having the same language for all packages.

If this assumption is not verified, we could work around it by:

  • recommending the users use withr:
    withr::with_language("es", epiCo:: ...)
  • using the proposed infrastructure here to handle translations without code duplication but keep the language argument and use withr::local_language("es") internally within the epiCo source code

@juan-umana
Copy link
Member

@Juanmontenegro99 I think the changed files are ok, as well as the translatons.

However, @Bisaloo, we have not been able to get the plot in the system configuration (spanish)

Here is an screenshot of R messages being displayed in spanish (after using the Sys.setlocale("LC_ALL", "es_ES") and/or withr::local_language("es"))

image

Could you help us reviewing the po files? Thanks!

@Bisaloo
Copy link
Member Author

Bisaloo commented Sep 2, 2024

Thanks for the report! It should be fixed with 22a77fb (#140).

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.

Use standard mechanisms to offer language options
3 participants