remove calls to pacta.portfolio.report:::translate_df_contents("data_key_bars_portfolio", dictionary)

[jdhoffa]

The dashboard (at least currently) is only going to be offered in EN, so it doesn't seem like this translation step is actually necessary?

@MonikaFu what do you think?

[MonikaFu]

In principle I agree. I was thinking about it as well. We can always bring it in if needed.

[jdhoffa]

Note to self, we will need to preserve the output data-frame to ensure compatibility with the dashboard.

• Check if the column names of the data-frame are identical before and after translation

[MonikaFu]

ah, good call, I believe they are not actually. The translation also changes some of the variables in English and for example - makes them more verbose. We also use the *_translated columns within plots.

[MonikaFu]

We should keep the step for now. The changes required to remove it would not be worth it. And we would still need some mechanism to extend some of the labels.

[jdhoffa]

@MonikaFu thanks for the input! but there are slightly more things to consider 😊

Currently we depend on a lot of internal functions from pacta.portfolio.report, which is not suitable for production software. This issue works towards removing that dependency.

If we remove that dependency, we will need to recode that functionality either way, so if we are "abusing" the translation function just to reformat a few columns, then I think that is overkill

[jdhoffa]

Relates to #29

[MonikaFu]

Fair. I meant more that at the moment it's not straightforward to just remove it. But we should potentially implement a more robust solution long-term.

[cjyetman]

I think this is resolved by #30 since all the pacta.portfolio.report were transferred here internally.

[jdhoffa]

The spirit of this issue is to actually remove the translate_df_contents functionality altogether, as translation is superfluous.

And this still exists in the repo: https://github.com/RMI-PACTA/workflow.pacta.dashboard/blob/main/R/translate_df_contents.R

So this issue should be left open.

[cjyetman]

So, I don't really see the point in removing the translate_df_contents() function. Surely, for how it's being used here it looks over engineered, but it's still doing something that has to be done one way or another... it converts "in-data" values to proper display values, e.g. "NuclearCap" -> "Nuclear Power", "Oil&Gas" -> "Oil & Gas", "Electric_HDV" -> "HDV: Electric" (see here for many more examples).

If you want to reduce the complexity of it, we could probably get away with removing the additional language columns from dataframe_labels.csv without much hassle. Obviously we could change the function name.

[cjyetman]

to be fair, the functionality does appear to be spread across 🤣:
https://github.com/RMI-PACTA/workflow.pacta.dashboard/tree/main/inst/extdata/translation
https://github.com/RMI-PACTA/workflow.pacta.dashboard/blob/main/R/translate_df_contents.R
https://github.com/RMI-PACTA/workflow.pacta.dashboard/blob/main/R/translate_column_contents.R
https://github.com/RMI-PACTA/workflow.pacta.dashboard/blob/main/R/translate_df_headers.R
https://github.com/RMI-PACTA/workflow.pacta.dashboard/blob/main/R/choose_dictionary_language.R
https://github.com/RMI-PACTA/workflow.pacta.dashboard/blob/main/R/replace_contents.R

[jdhoffa]

That is sort of my point ahah. For the interactive report, translation likely involved a bit more of an involved process.

I thin we can simplify this whole function immensely if the only purpose is to do simple label formatting prior to plotting...