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

print should not strip commodity from zero amounts when it is specified in the journal #475

Closed
peti opened this Issue Jan 13, 2017 · 26 comments

Comments

Projects
None yet
3 participants
@peti
Contributor

peti commented Jan 13, 2017

hledger print (and other commands) should not reduce EUR 0 to 0.

[context: #471]

[note: some discussion below is from @simonmichael thinking this issue was about making all commands show commodified zero]

@simonmichael simonmichael added the A WISH label Jan 13, 2017

@ony

This comment has been minimized.

Show comment
Hide comment
@ony

ony Jan 14, 2017

Collaborator

I guess this is a big deal and needs fixing...

; bug.journal
2017/1/1
    assets
    expenses  $5

2017/1/1
    assets  = $0
    expenses  = $0
% COLUMNS=80 hledger -f bug.journal reg 
2017/01/01                      assets                         $-5           $-5
                                expenses                        $5             0
2017/01/01                      assets                          $5            $5
                                expenses                       $-5             0
% hledger -f bug.journal print | COLUMNS=80 hledger -f - reg           
2017/01/01                      assets                         $-5           $-5
                                expenses                        $5             0
2017/01/01                      assets                           0             0
                                expenses                         0             0
Collaborator

ony commented Jan 14, 2017

I guess this is a big deal and needs fixing...

; bug.journal
2017/1/1
    assets
    expenses  $5

2017/1/1
    assets  = $0
    expenses  = $0
% COLUMNS=80 hledger -f bug.journal reg 
2017/01/01                      assets                         $-5           $-5
                                expenses                        $5             0
2017/01/01                      assets                          $5            $5
                                expenses                       $-5             0
% hledger -f bug.journal print | COLUMNS=80 hledger -f - reg           
2017/01/01                      assets                         $-5           $-5
                                expenses                        $5             0
2017/01/01                      assets                           0             0
                                expenses                         0             0
@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 14, 2017

Owner

Thanks for the example. Hmm, what exactly is going on here. Your file (1):

$ cat examples/475.journal
; bug.journal
2017/1/1
    assets
    expenses  $5

2017/1/1
    assets  = $0
    expenses  = $0

The expected balance report (2):

~/src/hledger$ hledger -f examples/475.journal bal -E
                   0  assets
                   0  expenses
--------------------
                   0

Output of print --explicit (3):

~/src/hledger$ hledger -f examples/475.journal print --explicit
2017/01/01
    assets             $-5
    expenses            $5

2017/01/01
    assets              $5 = 0
    expenses           $-5 = 0

The balance report is unchanged, as expected (4):

~/src/hledger$ hledger -f examples/475.journal print --explicit | hledger -f- bal -E
                   0  assets
                   0  expenses
--------------------
                   0

Ordinary style-preserving print (5):

~/src/hledger$ hledger -f examples/475.journal print
2017/01/01
    assets
    expenses            $5

2017/01/01
    assets     = 0
    expenses   = 0

That changes the balance report to something different (6):

~/src/hledger$ hledger -f examples/475.journal print | hledger -f- bal -E
                 $-5  assets
                  $5  expenses
--------------------
                   0

3 is actually incorrect, the assertions have been altered (= $0 and = 0 have a different meaning). With --explicit it's harmless, but in 5 it causes the posting amounts to be 0.

Owner

simonmichael commented Jan 14, 2017

Thanks for the example. Hmm, what exactly is going on here. Your file (1):

$ cat examples/475.journal
; bug.journal
2017/1/1
    assets
    expenses  $5

2017/1/1
    assets  = $0
    expenses  = $0

The expected balance report (2):

~/src/hledger$ hledger -f examples/475.journal bal -E
                   0  assets
                   0  expenses
--------------------
                   0

Output of print --explicit (3):

~/src/hledger$ hledger -f examples/475.journal print --explicit
2017/01/01
    assets             $-5
    expenses            $5

2017/01/01
    assets              $5 = 0
    expenses           $-5 = 0

The balance report is unchanged, as expected (4):

~/src/hledger$ hledger -f examples/475.journal print --explicit | hledger -f- bal -E
                   0  assets
                   0  expenses
--------------------
                   0

Ordinary style-preserving print (5):

~/src/hledger$ hledger -f examples/475.journal print
2017/01/01
    assets
    expenses            $5

2017/01/01
    assets     = 0
    expenses   = 0

That changes the balance report to something different (6):

~/src/hledger$ hledger -f examples/475.journal print | hledger -f- bal -E
                 $-5  assets
                  $5  expenses
--------------------
                   0

3 is actually incorrect, the assertions have been altered (= $0 and = 0 have a different meaning). With --explicit it's harmless, but in 5 it causes the posting amounts to be 0.

@ony

This comment has been minimized.

Show comment
Hide comment
@ony

ony Jan 14, 2017

Collaborator

@simonmichael with --explicit it still fails to perform assertion effectively.

Collaborator

ony commented Jan 14, 2017

@simonmichael with --explicit it still fails to perform assertion effectively.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 14, 2017

Owner

If I understand you - no, the = 0 assertion is being processed normally and succeeding (because assertions test only a single commodity's balance, following Ledger. We've had some debate about this in the past).

Owner

simonmichael commented Jan 14, 2017

If I understand you - no, the = 0 assertion is being processed normally and succeeding (because assertions test only a single commodity's balance, following Ledger. We've had some debate about this in the past).

@ony

This comment has been minimized.

Show comment
Hide comment
@ony

ony Jan 14, 2017

Collaborator
zsh% hledger -f - bal
2017/01/01
    assets  = $5
    equity

2017/01/01
    assets  $0 = 0

Output:

                  $5  assets
                 $-5  equity
--------------------
                   0

I'd expect assets $0 = 0 to fail.

Collaborator

ony commented Jan 14, 2017

zsh% hledger -f - bal
2017/01/01
    assets  = $5
    equity

2017/01/01
    assets  $0 = 0

Output:

                  $5  assets
                 $-5  equity
--------------------
                   0

I'd expect assets $0 = 0 to fail.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 14, 2017

Owner

= 0 tests that the balance in the null commodity (only) is zero, which it is as we've posted no null-commodity amounts. The dollar balance isn't asserted in the second transaction. Confusing isn't it. See http://hledger.org/journal.html#assertions-and-commodities.

Owner

simonmichael commented Jan 14, 2017

= 0 tests that the balance in the null commodity (only) is zero, which it is as we've posted no null-commodity amounts. The dollar balance isn't asserted in the second transaction. Confusing isn't it. See http://hledger.org/journal.html#assertions-and-commodities.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 17, 2017

Owner

Update: a related print bug was fixed by commit 8bc67d5 above; the larger wishlist issue of making all commands show commodified zero remains open, and needs experimentation & testing.

Owner

simonmichael commented Jan 17, 2017

Update: a related print bug was fixed by commit 8bc67d5 above; the larger wishlist issue of making all commands show commodified zero remains open, and needs experimentation & testing.

@simonmichael simonmichael changed the title from EUR 0 /= 0 to commands should not strip commodity when displaying zero amounts Jan 17, 2017

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 17, 2017

Owner

..and I for one do not yet fully understand the meaning of a "commodified zero". Is it a real thing ?

Owner

simonmichael commented Jan 17, 2017

..and I for one do not yet fully understand the meaning of a "commodified zero". Is it a real thing ?

@peti

This comment has been minimized.

Show comment
Hide comment
@peti

peti Jan 17, 2017

Contributor

In my mind, commodities like EUR and USD are base units for different dimensions, just like kg and m are base units for the dimensions mass and length. Thus, EUR 0 and USD 0 are fundamentally different quantities, just like 0 m and 0 kg are.

Contributor

peti commented Jan 17, 2017

In my mind, commodities like EUR and USD are base units for different dimensions, just like kg and m are base units for the dimensions mass and length. Thus, EUR 0 and USD 0 are fundamentally different quantities, just like 0 m and 0 kg are.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 18, 2017

Owner

Is it something accountants recognise ? How urgent is this feature for hledger users ? I don't currently have any good use cases in mind (except for the hledger-web chart, where it helped show separate trend line for each commodity).

Owner

simonmichael commented Jan 18, 2017

Is it something accountants recognise ? How urgent is this feature for hledger users ? I don't currently have any good use cases in mind (except for the hledger-web chart, where it helped show separate trend line for each commodity).

@peti

This comment has been minimized.

Show comment
Hide comment
@peti

peti Jan 18, 2017

Contributor

I don't know about accountants, unfortunatly. For me, personally, what is important is that hledger print does not lose the commodity, i.e. in a balance assertion, so that I can use that process to normalize and to pretty-print my journal without losing information I'd rather keep. It's not a big deal, but I'd prefer my balance assertion to say = EUR 0, not = 0.

Contributor

peti commented Jan 18, 2017

I don't know about accountants, unfortunatly. For me, personally, what is important is that hledger print does not lose the commodity, i.e. in a balance assertion, so that I can use that process to normalize and to pretty-print my journal without losing information I'd rather keep. It's not a big deal, but I'd prefer my balance assertion to say = EUR 0, not = 0.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 18, 2017

Owner

Ah! Well that (losing the commodity from assertions) was definitely a bug, I also noticed that and fixed it (see above). I thought this issue was about displaying zeroes with commodities generally, eg in reports.

Owner

simonmichael commented Jan 18, 2017

Ah! Well that (losing the commodity from assertions) was definitely a bug, I also noticed that and fixed it (see above). I thought this issue was about displaying zeroes with commodities generally, eg in reports.

@peti

This comment has been minimized.

Show comment
Hide comment
@peti

peti Jan 18, 2017

Contributor

It seems like the currency is still lost from conversions, though: NVDA 46 @ EUR 0 becomes NVDA 46 @ 0 during printing.

Contributor

peti commented Jan 18, 2017

It seems like the currency is still lost from conversions, though: NVDA 46 @ EUR 0 becomes NVDA 46 @ 0 during printing.

@peti

This comment has been minimized.

Show comment
Hide comment
@peti

peti Jan 18, 2017

Contributor

print also still drops the specified currency from pamounts by default. That's less of a problem, IMHO, because posting amounts of zero make little sense in general, but still it would be great if print would retain the information I gave in the journal.

Contributor

peti commented Jan 18, 2017

print also still drops the specified currency from pamounts by default. That's less of a problem, IMHO, because posting amounts of zero make little sense in general, but still it would be great if print would retain the information I gave in the journal.

@ony

This comment has been minimized.

Show comment
Hide comment
@ony

ony Jan 18, 2017

Collaborator

Looks like showAmount simplifies zeroes starting from bfe935e18:

fix a sign bug with negative total-priced (@@) amounts

 -- | Get the string representation of an amount, based on its commodity's
--- display settings.
+-- display settings. Amounts which look like zero are rendered without sign or commodity.
 showAmount :: Amount -> String
 showAmount (Amount (Commodity {symbol="AUTO"}) _ _) = "" -- can appear in an error message
 showAmount a@(Amount (Commodity {symbol=sym,side=side,spaced=spaced}) _ pri) =
     case side of
-      L -> printf "%s%s%s%s" sym' space quantity price
+      L -> printf "%s%s%s%s" sym' space quantity' price
-      R -> printf "%s%s%s%s" quantity space sym' price
+      R -> printf "%s%s%s%s" quantity' space sym' price
     where
-      sym' = quoteCommoditySymbolIfNeeded sym
-      space = if (spaced && not (null sym')) then " " else ""
-      quantity = showAmount' a
+      quantity = showamountquantity a
+      displayingzero = null $ filter (`elem` "123456789") $ quantity
+      (quantity',sym') | displayingzero = ("0","")
+                       | otherwise      = (quantity,quoteCommoditySymbolIfNeeded sym)
+      space = if (not (null sym') && spaced) then " " else ""
       price = maybe "" showPrice pri

@simonmichael, do we still need this behavior? Maybe we can simply drop it and always render amounts with currencies and associated format style?

Collaborator

ony commented Jan 18, 2017

Looks like showAmount simplifies zeroes starting from bfe935e18:

fix a sign bug with negative total-priced (@@) amounts

 -- | Get the string representation of an amount, based on its commodity's
--- display settings.
+-- display settings. Amounts which look like zero are rendered without sign or commodity.
 showAmount :: Amount -> String
 showAmount (Amount (Commodity {symbol="AUTO"}) _ _) = "" -- can appear in an error message
 showAmount a@(Amount (Commodity {symbol=sym,side=side,spaced=spaced}) _ pri) =
     case side of
-      L -> printf "%s%s%s%s" sym' space quantity price
+      L -> printf "%s%s%s%s" sym' space quantity' price
-      R -> printf "%s%s%s%s" quantity space sym' price
+      R -> printf "%s%s%s%s" quantity' space sym' price
     where
-      sym' = quoteCommoditySymbolIfNeeded sym
-      space = if (spaced && not (null sym')) then " " else ""
-      quantity = showAmount' a
+      quantity = showamountquantity a
+      displayingzero = null $ filter (`elem` "123456789") $ quantity
+      (quantity',sym') | displayingzero = ("0","")
+                       | otherwise      = (quantity,quoteCommoditySymbolIfNeeded sym)
+      space = if (not (null sym') && spaced) then " " else ""
       price = maybe "" showPrice pri

@simonmichael, do we still need this behavior? Maybe we can simply drop it and always render amounts with currencies and associated format style?

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 18, 2017

Owner

Not sure! Let's try it.

I would like come up with a clear theory/story for when showing commodity for zero makes sense. Otherwise there's the chance that with each little change like this we get deeper into a big destabilising/de-ledger-compatibilising task, the full size of which is not yet clear, and with the chance of having to rethink and backtrack on some of these decisions.

Owner

simonmichael commented Jan 18, 2017

Not sure! Let's try it.

I would like come up with a clear theory/story for when showing commodity for zero makes sense. Otherwise there's the chance that with each little change like this we get deeper into a big destabilising/de-ledger-compatibilising task, the full size of which is not yet clear, and with the chance of having to rethink and backtrack on some of these decisions.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 19, 2017

Owner

So our current zero story is "zero is considered commodityless, and we display it without a commodity (except for a special case in hledger-web)". This is similar to Ledger.

A new story could be, for example: "zero can have a commodity, eg because specified by a journal entry or inherited from summed amounts, and we preserve and display that." This change might be very easy, or it might raise new issues; testing required.

What's the real problem it would solve ? I'm guessing the main goal is to firm up our new print story: "print preserves as much information from the journal as possible". @peti is this issue about that, or a general wish to track and report zero's commodity everywhere ? We should probably have separate issues for these.

Owner

simonmichael commented Jan 19, 2017

So our current zero story is "zero is considered commodityless, and we display it without a commodity (except for a special case in hledger-web)". This is similar to Ledger.

A new story could be, for example: "zero can have a commodity, eg because specified by a journal entry or inherited from summed amounts, and we preserve and display that." This change might be very easy, or it might raise new issues; testing required.

What's the real problem it would solve ? I'm guessing the main goal is to firm up our new print story: "print preserves as much information from the journal as possible". @peti is this issue about that, or a general wish to track and report zero's commodity everywhere ? We should probably have separate issues for these.

@peti

This comment has been minimized.

Show comment
Hide comment
@peti

peti Jan 19, 2017

Contributor

Yes, as I said, I don't care if currencies are dropped from zeros in the reports. My interest is in print keeping the currencies I specified in the generated journal.

Contributor

peti commented Jan 19, 2017

Yes, as I said, I don't care if currencies are dropped from zeros in the reports. My interest is in print keeping the currencies I specified in the generated journal.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 19, 2017

Owner

Aha. This was not clear to me. I'll update this issue's title.

Owner

simonmichael commented Jan 19, 2017

Aha. This was not clear to me. I'll update this issue's title.

@simonmichael simonmichael changed the title from commands should not strip commodity when displaying zero amounts to print should not strip commodity from zero amounts when it is specified in the journal Jan 19, 2017

@simonmichael simonmichael added A BUG journal print and removed A WISH labels Jan 19, 2017

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 19, 2017

Owner

Ok. Please add reproducible examples of remaining cases where print strips commodity from zeros, as you find them.

Owner

simonmichael commented Jan 19, 2017

Ok. Please add reproducible examples of remaining cases where print strips commodity from zeros, as you find them.

@peti

This comment has been minimized.

Show comment
Hide comment
@peti

peti Jan 22, 2017

Contributor

I'm using hledger print now all the time to convert my journal into a "canonic" format, and it works great. If I come across any cases where information is lost, then I'll open a new ticket. Thank you very much for the great work, guys!

Contributor

peti commented Jan 22, 2017

I'm using hledger print now all the time to convert my journal into a "canonic" format, and it works great. If I come across any cases where information is lost, then I'll open a new ticket. Thank you very much for the great work, guys!

@peti peti closed this Jan 22, 2017

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 22, 2017

Owner

Thanks! There are two known things not preserved: 1. directives and 2. inter-transaction comments. I suppose those should be printed too.

Owner

simonmichael commented Jan 22, 2017

Thanks! There are two known things not preserved: 1. directives and 2. inter-transaction comments. I suppose those should be printed too.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 22, 2017

Owner

..which requires a little thought. Do you always want those in the output (print is used for reporting, small-scale reformatting, whole-journal transformation, and export), what if there are query options/arguments, etc.

Owner

simonmichael commented Jan 22, 2017

..which requires a little thought. Do you always want those in the output (print is used for reporting, small-scale reformatting, whole-journal transformation, and export), what if there are query options/arguments, etc.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 22, 2017

Owner

What about: directives and file-level comments are shown when you print with no filtering of any kind; otherwise only transactions are shown.

Owner

simonmichael commented Jan 22, 2017

What about: directives and file-level comments are shown when you print with no filtering of any kind; otherwise only transactions are shown.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 22, 2017

Owner

Also, when does print show the directive for "automated" things (modified transactions, periodic transactions) and when does it show the modified/generated transactions instead.

Owner

simonmichael commented Jan 22, 2017

Also, when does print show the directive for "automated" things (modified transactions, periodic transactions) and when does it show the modified/generated transactions instead.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 22, 2017

Owner

Ledger has (and I'll stop..):

     --raw    In the print report, show transactions using the exact same syn-
              tax as specified by the user in their data file.  Don't do any
              massaging or interpreting.  Can be useful for minor cleanups,
              like just aligning amounts.
Owner

simonmichael commented Jan 22, 2017

Ledger has (and I'll stop..):

     --raw    In the print report, show transactions using the exact same syn-
              tax as specified by the user in their data file.  Don't do any
              massaging or interpreting.  Can be useful for minor cleanups,
              like just aligning amounts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment