Skip to content

A minus sign before the currency reads as negative, so "-$5 million" is -5000000 instead of no value - #1435

Open
rudycelekli wants to merge 1 commit into
deeplethe:devfrom
rudycelekli:extract-minus-before-currency
Open

rudycelekli wants to merge 1 commit into
deeplethe:devfrom
rudycelekli:extract-minus-before-currency

Conversation

@rudycelekli

Copy link
Copy Markdown
Contributor

Why

scan_quantity reads a currency symbol at the very start of the text, then the number with its sign. A loss or a negative balance is usually written with the minus in front of the symbol: "-$5 million", "−€30 million", "-$1,250". There the first character is -, so no currency is found, the number scan stops at $, and the value is not read at all. The same figure written "$-5 million" reads as -5000000.

So a net loss stated as "-$5 million" under a number attribute stays out of the typed graph, and literal_object does not see it as a value either.

What changes

If the text starts with - or the typeset − and the next character is a currency symbol, the sign is set aside, the currency is read as before, and the sign goes back on the number before it is parsed (so a magnitude word still scales the signed value). A dash that is not followed by a currency symbol is left alone, so "- 5" is still not a number, and a double sign ("-$-5") is still no value.

How it was checked

  • A new test, a_minus_sign_before_the_currency_is_read_as_negative, covers "-$5 million", "−€30 million", "-$1,250", "-£0.5", "-¥2.3亿" under 万元, "- 5" and "-$-5".
  • With the fix reverted the test fails: left: None right: Some(Number(-5000000.0)).
  • cargo test -p utopia-extract --lib: 113 passed.
  • cargo fmt -p utopia-extract --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

…is -5000000 instead of no value

Signed-off-by: Rudy Celekli <rudy@gradiahq.com>
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.

1 participant