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

Support for Commodity equivalences #678

Open
rainbyte opened this Issue Dec 22, 2017 · 12 comments

Comments

Projects
None yet
4 participants
@rainbyte
Contributor

rainbyte commented Dec 22, 2017

I'm having trouble migrating a ledger-cli journal to hledger, which uses C directive.

My file is similar to this one (but contains more transactions)

commodity mBTC
commodity BTC
commodity ARS

C 1 BTC = 1000 mBTC

2017/12/22 saldo inicial
    activo:bitcoin wallet a    3 BTC
    activo:bitcoin wallet b    2 BTC
    activo:efectivo         1000 ARS
    saldo inicial

2017/12/22 compra bitcoins 1
    activo:bitcoin wallet a    1000 mBTC
    gastos:comisiones             1 mBTC
    activo:bitcoin wallet b   -1.001 BTC

; This is the problematic transaction
2017/12/22 compra bitcoins 2
    activo:bitcoin wallet a    1000 mBTC
    gastos:comisiones            260 ARS @@ 1 mBTC
    activo:bitcoin wallet b   -1.001 BTC

The error:

hledger: could not balance this transaction (real postings are off by -1.001 BTC
 1001 mBTC)
2017/12/22 compra bitcoins 2
    activo:bitcoin wallet a            1000 mBTC
    gastos:comisiones          260 ARS @@ 1 mBTC
    activo:bitcoin wallet b           -1.001 BTC

It seems hledger has problems to handle balance for last transaction, which is valid on ledger-cli.

Also, if last transaction is commented, balance shows BTC and mBTC separated.

Early I have cloned the repo and found that the is a commodityconversiondirectivep function.
But this seems to be an unimplemented feature after all.

I'm whiling to write the missing code if a bit of guiding is given.

Thanks!

@rainbyte

This comment has been minimized.

Show comment
Hide comment
@rainbyte

rainbyte Dec 22, 2017

Contributor

This feature is mentioned here inside ledger-cli docs page

Contributor

rainbyte commented Dec 22, 2017

This feature is mentioned here inside ledger-cli docs page

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Dec 22, 2017

Owner

@rainbyte that's right, we don't have that feature yet. Partly because we are trying to reduce feature count, partly because nobody needed it or did it yet.

I minified the example some more:

; https://github.com/simonmichael/hledger/issues/678

; hledger accepts but ignores this
C 1 BTC = 1000 mBTC

; hledger accepts these, treating BTC and mBTC as separate commodities, -B required to combine them

2017/12/1
    a    1000 mBTC
    b       -1 BTC

2017/12/2
    a    1000 mBTC
    c       1 mBTC
    b   -1.001 BTC

2017/12/3
    a    1000 mBTC
    c       1 mBTC
    b   -1.001 BTC

2017/12/4
    a     1000 mBTC
    c      260 ARS @@ 1 mBTC
    b    -1001 mBTC

; hledger does not understand this:

2017/12/5
    a    1000 mBTC
    c      260 ARS @@ 1 mBTC  ; or @@ 0.001 BTC
    b   -1.001 BTC

As you see from the comments, it treats them as separate commodities, for which -B may be a workaround. But it gets confused by the last transaction, which it probably should not.

Owner

simonmichael commented Dec 22, 2017

@rainbyte that's right, we don't have that feature yet. Partly because we are trying to reduce feature count, partly because nobody needed it or did it yet.

I minified the example some more:

; https://github.com/simonmichael/hledger/issues/678

; hledger accepts but ignores this
C 1 BTC = 1000 mBTC

; hledger accepts these, treating BTC and mBTC as separate commodities, -B required to combine them

2017/12/1
    a    1000 mBTC
    b       -1 BTC

2017/12/2
    a    1000 mBTC
    c       1 mBTC
    b   -1.001 BTC

2017/12/3
    a    1000 mBTC
    c       1 mBTC
    b   -1.001 BTC

2017/12/4
    a     1000 mBTC
    c      260 ARS @@ 1 mBTC
    b    -1001 mBTC

; hledger does not understand this:

2017/12/5
    a    1000 mBTC
    c      260 ARS @@ 1 mBTC  ; or @@ 0.001 BTC
    b   -1.001 BTC

As you see from the comments, it treats them as separate commodities, for which -B may be a workaround. But it gets confused by the last transaction, which it probably should not.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Dec 22, 2017

Owner

PS so on the question of whether the above failure is a bug in the current feature set.. We could make the (complex) transaction balancing code more clever, and handle cases like the above. Will that be a final fix, or will there always be more complex ways of adding prices and confusing it ? I'm not sure.

Owner

simonmichael commented Dec 22, 2017

PS so on the question of whether the above failure is a bug in the current feature set.. We could make the (complex) transaction balancing code more clever, and handle cases like the above. Will that be a final fix, or will there always be more complex ways of adding prices and confusing it ? I'm not sure.

@rainbyte

This comment has been minimized.

Show comment
Hide comment
@rainbyte

rainbyte Dec 22, 2017

Contributor

First, thanks for the fast answer 👍

I think equivalences feature would be useful given the usage of cryptocurrencies today.

At least cases like this one should be supported:

C 1 BTC = 1000 mBTC
C 1 mBTC = 1000 uBTC
C 1 sat = 0.00001 mBTC

It is really easy to get confused when writing various zeros (eg. 0.0001 BTC vs 0.1 mBTC).

In my case I prefer using mBTC myself, and BTC only via copy-paste from exchange.

I'm reading the hledger code (and also ledger-cli code to see how it is done there).

Let's see if I can contribute something to hledger :D

Contributor

rainbyte commented Dec 22, 2017

First, thanks for the fast answer 👍

I think equivalences feature would be useful given the usage of cryptocurrencies today.

At least cases like this one should be supported:

C 1 BTC = 1000 mBTC
C 1 mBTC = 1000 uBTC
C 1 sat = 0.00001 mBTC

It is really easy to get confused when writing various zeros (eg. 0.0001 BTC vs 0.1 mBTC).

In my case I prefer using mBTC myself, and BTC only via copy-paste from exchange.

I'm reading the hledger code (and also ledger-cli code to see how it is done there).

Let's see if I can contribute something to hledger :D

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Dec 22, 2017

Owner

Ok, good. To avoid wasted work, I often draft the docs first (a reference manual entry). This helps me get the semantics clear. I haven't used this feature, and it might be explained in the Ledger manual, but it will be good to have a doc explaining

  • how this behaves differently from separate commodities, and when it's better to use one or the other
  • how hledger decides which of the equivalent units to display
Owner

simonmichael commented Dec 22, 2017

Ok, good. To avoid wasted work, I often draft the docs first (a reference manual entry). This helps me get the semantics clear. I haven't used this feature, and it might be explained in the Ledger manual, but it will be good to have a doc explaining

  • how this behaves differently from separate commodities, and when it's better to use one or the other
  • how hledger decides which of the equivalent units to display
@lordi

This comment has been minimized.

Show comment
Hide comment
@lordi

lordi Mar 9, 2018

My use case for this feature is time tracking, as mentioned in the ledger docs. Currently the reports generated with hledger show hours and minutes as different commodities, compared to those generated with ledger which show minutes when below 1 h, and hours otherwise.

lordi commented Mar 9, 2018

My use case for this feature is time tracking, as mentioned in the ledger docs. Currently the reports generated with hledger show hours and minutes as different commodities, compared to those generated with ledger which show minutes when below 1 h, and hours otherwise.

@ony

This comment has been minimized.

Show comment
Hide comment
@ony

ony Jun 7, 2018

Collaborator

I also got a use-case for it to track traffic KB/MB/GB/etc...
Probably we still need commodity directive to have default representation. But on the other hand it is completely useless to show 0.000 GB. And showing all numbers in the smallest units also tedious while having a "smart" adaptation and seeing mix in balance might be even worse.

"smart" choose of unit might be reasonable based on whole report. Like get the biggest one which doesn't render non-zero values to zero.
Also as shown above it makes sense to have style (mostly for precision) to be associated with each unit.

So my suggestion:

  • commodity declares base commodity.
  • C declares how other units of same commodity can be deduced/interpreted. Left-hand side should refer either to existing commodity directive or right-hand side of other C directive with cycles being identified and reported as invalid journal.
  • Within all mentioned commodity symbols/names in tree built out of C directives there only one allowed to be also declared with commodity directive.
  • During report by default we use commodity in base unit (declared with commodity directive).
  • Additionally we may add reporting option to have "smart" units that choose as "base unit" the one that strictly doesn't hide numbers behind complete zero digits (0.0 GB) and those that makes at least one number to be greater or equal to one (ex 1.1 MB) if possible.
  • All units of share same style for digit groups and their separator as per commodity directive.
  • Precision and decimal separator is taken from C directives and should not clash with digit groups separator in commodity.

For simplicity we may forbid tree of conversion and enforce only chain:

C 1 X = 1000 Y
C 1 X = 100 Z
C 10 K = 1 Z  ; ambigous 1 X can be either 1000 Y or 1000 K
Collaborator

ony commented Jun 7, 2018

I also got a use-case for it to track traffic KB/MB/GB/etc...
Probably we still need commodity directive to have default representation. But on the other hand it is completely useless to show 0.000 GB. And showing all numbers in the smallest units also tedious while having a "smart" adaptation and seeing mix in balance might be even worse.

"smart" choose of unit might be reasonable based on whole report. Like get the biggest one which doesn't render non-zero values to zero.
Also as shown above it makes sense to have style (mostly for precision) to be associated with each unit.

So my suggestion:

  • commodity declares base commodity.
  • C declares how other units of same commodity can be deduced/interpreted. Left-hand side should refer either to existing commodity directive or right-hand side of other C directive with cycles being identified and reported as invalid journal.
  • Within all mentioned commodity symbols/names in tree built out of C directives there only one allowed to be also declared with commodity directive.
  • During report by default we use commodity in base unit (declared with commodity directive).
  • Additionally we may add reporting option to have "smart" units that choose as "base unit" the one that strictly doesn't hide numbers behind complete zero digits (0.0 GB) and those that makes at least one number to be greater or equal to one (ex 1.1 MB) if possible.
  • All units of share same style for digit groups and their separator as per commodity directive.
  • Precision and decimal separator is taken from C directives and should not clash with digit groups separator in commodity.

For simplicity we may forbid tree of conversion and enforce only chain:

C 1 X = 1000 Y
C 1 X = 100 Z
C 10 K = 1 Z  ; ambigous 1 X can be either 1000 Y or 1000 K
@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jun 7, 2018

Owner

Thanks for the draft spec. I'm not sure we have to worry about "0.000 GB" - if the user has configured things that way perhaps they want it to appear as "zero".

I'm concerned about complexity and the long tail of bugs that may follow. Any benefit in an alternate syntax using subdirectives of commodity ?

Owner

simonmichael commented Jun 7, 2018

Thanks for the draft spec. I'm not sure we have to worry about "0.000 GB" - if the user has configured things that way perhaps they want it to appear as "zero".

I'm concerned about complexity and the long tail of bugs that may follow. Any benefit in an alternate syntax using subdirectives of commodity ?

@ony

This comment has been minimized.

Show comment
Hide comment
@ony

ony Jun 8, 2018

Collaborator

@simonmichael, do you mean that we can require something like:

commodity 1,000,000,000 B
  unit 1.0 KB = 1024 B  ; inherits base but overrides for KB precision
  unit 1.000 MB = 1,048,576 B  ; right hand side always in base unit?

Then hledger bal --preferred-unit MB --preferred-unit minute.

Collaborator

ony commented Jun 8, 2018

@simonmichael, do you mean that we can require something like:

commodity 1,000,000,000 B
  unit 1.0 KB = 1024 B  ; inherits base but overrides for KB precision
  unit 1.000 MB = 1,048,576 B  ; right hand side always in base unit?

Then hledger bal --preferred-unit MB --preferred-unit minute.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jun 9, 2018

Owner

Yes, for example. If it turned out to lead to something clearer and more usable. Since I suppose we shouldn't let tradition and compatibility stop us from evolving.

But we'd probably want to support Ledger-style C as well, at least for a while. Or provide a from-Ledger conversion tool. Or, add a Ledger-specific parser, again.

Owner

simonmichael commented Jun 9, 2018

Yes, for example. If it turned out to lead to something clearer and more usable. Since I suppose we shouldn't let tradition and compatibility stop us from evolving.

But we'd probably want to support Ledger-style C as well, at least for a while. Or provide a from-Ledger conversion tool. Or, add a Ledger-specific parser, again.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jun 9, 2018

Owner

Also "C" always seems non-mnemonic to me - what does it stand for ? We have some directives that are words, some that are single letters; it would be nice to be more consistent some day, eg by allowing a short and long form of every directive.

Owner

simonmichael commented Jun 9, 2018

Also "C" always seems non-mnemonic to me - what does it stand for ? We have some directives that are words, some that are single letters; it would be nice to be more consistent some day, eg by allowing a short and long form of every directive.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jun 9, 2018

Owner

In fact it's quite a pity C isn't available as shorthand for "commodity"!

Owner

simonmichael commented Jun 9, 2018

In fact it's quite a pity C isn't available as shorthand for "commodity"!

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