Replace language arg by standard translation mechanism

epiverse-trace / epiCo

R package for statistical and spatiotemporal modeling of epidemiological data for vector-borne diseases in Colombia

https://epiverse-trace.github.io/epiCo/

Other

12 stars 1 forks source link

Replace language arg by standard translation mechanism #140

Closed Bisaloo closed 2 months ago

Bisaloo commented 3 months ago

Fix #138

Todo:

[x] Move the Spanish translations to the po/R-es.po file
[x] Possible remove the translation arg from describe_ethnicity() as well
[x] 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 commented 3 months ago

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 commented 3 months ago

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 commented 2 months ago

@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"))

Could you help us reviewing the po files? Thanks!

Bisaloo commented 2 months ago

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