Skip to content

Replace USD with CURRENCY_CODE in rp2_full_report.py#144

Open
archont00 wants to merge 2 commits into
eprbell:mainfrom
archont00:patch-1
Open

Replace USD with CURRENCY_CODE in rp2_full_report.py#144
archont00 wants to merge 2 commits into
eprbell:mainfrom
archont00:patch-1

Conversation

@archont00

Copy link
Copy Markdown

The sheet Summary of the full report had "USD" currency code hard-coded in the headings. Replacing with reference to CURRENCY_CODE instead. Tested with rp2_generic plugin.

archont00 and others added 2 commits October 19, 2025 11:00
The sheet Summary of the full report had "USD" currency code hard-coded in the headings. Replacing with reference to CURRENCY_CODE instead.
@macanudo527

Copy link
Copy Markdown
Collaborator

@archont00

Sorry about the delay, can you update the tests as well? I think you just need to update the golden files to reflect the change perhaps.

@macanudo527

macanudo527 commented May 29, 2026

Copy link
Copy Markdown
Collaborator

Hi @archont00 — thanks for this fix! While reviewing this PR we implemented the change along with unit tests verifying the header behavior for multiple non-USD currencies (EUR, GBP, JPY, CHF, CAD).

The work is on this branch: macanudo527/rp2claude/eloquent-euler-26eAX

The two relevant commits are:

  • Fix (src/rp2/plugin/report/rp2_full_report.py): same change as this PR — replaces _("USD") / _("USD Total") with _("{}").format(currency_code) / _("{} Total").format(currency_code)
  • Tests (tests/test_currency_code_headers.py): unit tests that directly call _setup_text_data() and assert headers contain the correct currency code (and do not contain a hardcoded "USD" for non-USD countries)

You're welcome to cherry-pick from that branch if you'd like to add the tests to your PR. Alternatively, if you'd prefer, we can open a new PR that supersedes this one — whichever works best for you and @eprbell.

I'll give you 2 weeks to respond and then I'll close this one down and supersede it.


Generated by Claude Code

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.

2 participants