New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simple prototype of scientific number notation support #706

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@ony
Collaborator

ony commented Feb 9, 2018

This is follow-up change for #704

@ony ony requested a review from simonmichael Feb 9, 2018

@wafflebot wafflebot bot assigned ony Feb 9, 2018

@wafflebot wafflebot bot added the in progress label Feb 9, 2018

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Mar 12, 2018

Owner

Looking good, with this patch hledger successfully parses coinbase CSV when it contains scientific notation. Nice tests. I expect some corner cases remain to be clarified like you mentioned on #704, but let's wait and see.

The "some strange effect of default" issue seems to be unrelated to this PR so could be removed, I have opened #716 for it.

This should be mentioned in Amounts; let me know if you want help with that.

Owner

simonmichael commented Mar 12, 2018

Looking good, with this patch hledger successfully parses coinbase CSV when it contains scientific notation. Nice tests. I expect some corner cases remain to be clarified like you mentioned on #704, but let's wait and see.

The "some strange effect of default" issue seems to be unrelated to this PR so could be removed, I have opened #716 for it.

This should be mentioned in Amounts; let me know if you want help with that.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Mar 25, 2018

@ony is this one a failing test ? It could use a comment. Shouldn't we expect this to pass ?

simonmichael commented on tests/journal/scientific.test in bb5aeb5 Mar 25, 2018

@ony is this one a failing test ? It could use a comment. Shouldn't we expect this to pass ?

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Mar 25, 2018

Oh I remember, you proposed to disallow digit groups in scientific numbers. But wikipedia says it's legal and it would be nice to be consistent with non-scientific amounts. Is it hard to support ?

simonmichael replied Mar 25, 2018

Oh I remember, you proposed to disallow digit groups in scientific numbers. But wikipedia says it's legal and it would be nice to be consistent with non-scientific amounts. Is it hard to support ?

This comment has been minimized.

Show comment
Hide comment
@ony

ony Mar 26, 2018

Owner

@simonmichael , it is more about ambiguity rather than complexity, though last one also present. Reason why I tend to ignore and/or disallow digit groups in scientific format is because it becomes misleading when you use exponent with money where scaling is a bit inappropriate. I.e. digit groups in numbers associated with currencies helps to understand magnitude of your expenses, but with something like 1 999.00e-2 you can't trust it anymore. And consider how to interpret 1.999 99e3?
Regarding complexity... So far we always assume that if we have at least two separators in a number, then the last one is decimal. But with digit groups after decimal point numbers like 1.999,99e3 also becomes ambiguous (either 1 999.99 or 1 999 990). The only cases where we'll be able to make a guess are: 0.999,99e3 (leading zero - currently unused), 1,999.990,000 (two and more digit groups separators). Thus we'll need to improve heuristic of guessing numbers further making it less comprehensible (less chances that someone else will touch it later).
Optionally if you want to use style from decimal digit grouping we'll need to extend AmountStyle (simple but potentially big code change).

Owner

ony replied Mar 26, 2018

@simonmichael , it is more about ambiguity rather than complexity, though last one also present. Reason why I tend to ignore and/or disallow digit groups in scientific format is because it becomes misleading when you use exponent with money where scaling is a bit inappropriate. I.e. digit groups in numbers associated with currencies helps to understand magnitude of your expenses, but with something like 1 999.00e-2 you can't trust it anymore. And consider how to interpret 1.999 99e3?
Regarding complexity... So far we always assume that if we have at least two separators in a number, then the last one is decimal. But with digit groups after decimal point numbers like 1.999,99e3 also becomes ambiguous (either 1 999.99 or 1 999 990). The only cases where we'll be able to make a guess are: 0.999,99e3 (leading zero - currently unused), 1,999.990,000 (two and more digit groups separators). Thus we'll need to improve heuristic of guessing numbers further making it less comprehensible (less chances that someone else will touch it later).
Optionally if you want to use style from decimal digit grouping we'll need to extend AmountStyle (simple but potentially big code change).

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Mar 26, 2018

(Continued back on #704.)

simonmichael replied Mar 26, 2018

(Continued back on #704.)

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Mar 31, 2018

Owner

Merged as b377bf etc., thanks.

Owner

simonmichael commented Mar 31, 2018

Merged as b377bf etc., thanks.

@wafflebot wafflebot bot removed the in progress label Mar 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment