Skip to content

A currency code written against its number is read, so "HK$300 million" is 300 million instead of no value - #1423

Open
rudycelekli wants to merge 1 commit into
deeplethe:devfrom
rudycelekli:extract-glued-currency-code
Open

rudycelekli wants to merge 1 commit into
deeplethe:devfrom
rudycelekli:extract-glued-currency-code

Conversation

@rudycelekli

Copy link
Copy Markdown
Contributor

Why

currency_unit knows HK$, US$, RMB, USD and the other multi-character currencies. scan_quantity only finds them in two places: as the very first character ($, €, ¥), or as a word followed by a space (USD 30).

Hong Kong and China filings usually write the code directly against the number: HK$300 million, US$5 billion, RMB5000万. For those, no currency is found and the scan stops at the first letter. The number attribute is then dropped. normalize_attr_value("number", "HK$300 million") is None, and so is attribute_value under 万元 for RMB5000万.

What changes

If neither existing check finds a currency, the text before the first digit is looked up in the same currency_unit table. HK$, US$, RMB and USD are found there, and the scan continues from the number. Text that is not in the table, like the A in A320, is not a currency, so a model number stays a name.

How it was checked

A new test, a_currency_code_written_against_the_number_is_read:

  • HK$300 million is 300000000.
  • US$5 billion is 5000000000.
  • RMB5000万 under 万元 is 5000.
  • US$5000万 under 万元 is still refused as a currency mismatch.
  • A320 is still not a literal.

Results:

  • cargo test -p utopia-extract --lib: 112 passed.
  • With the fix reverted, the test fails with left: None, right: Some(Number(300000000.0)).
  • cargo fmt --all --check and cargo clippy -p utopia-extract --all-targets -- -D warnings pass.

Before review

  • Every commit is signed off (git commit -s)
  • cargo fmt --all --check, cargo clippy and cargo test pass for the touched crate

…n" is 300 million instead of no value

Signed-off-by: Rudy Celekli <rudy@gradiahq.com>

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new branch also runs in the strict scan, so a named "USD1" or "euro2024" now counts as a value and its node is lost. Please limit it to the lenient scan, as #1422 does, and accept a - after the code so "HK$(300) million" works once #1422 lands. It conflicts with #1416 and #1422; please rebase after them.

@WaylandYang

Copy link
Copy Markdown
Contributor

Thanks for the steady stream of fixes; most of them are real and well tested. To keep review manageable, please answer the open change requests (#1398, #1399, #1403, #1405, #1407 and today's) before opening new PRs, and group fixes that touch the same function into one PR.

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