Ignore commodities without price

[aldur]

Some commodities in my ledger do not have a price, nor are meant to have one.

Before, I was getting the following error:

Error: unable to convert 1 XXX to base currency YYY (Missing price directive?)

As a result, fava was also exiting.

This PR simply ignores those commodities.

[redstreet]

Hmm, without price entries and converting them, the resulting asset allocation would be incorrect. This is why they are treated as errors and not warnings.

I would not want to change that unless there's a strong counter argument I'm missing. If there is one, perhaps a switch targeted towards such a scenario would be best. Thoughts?

[aldur]

That makes sense, thank you! What do you think of entirely ignoring unpriced assets from the overall asset allocation?

[redstreet]

That makes sense, thank you! What do you think of entirely ignoring unpriced assets from the overall asset allocation?

The problem with that is, we get an allocation output that looks correct, with no indication that certain assets were skipped.

If the goal is to tell the system to not include certain assets, doing so by implicitly excluding those without a price conversion would frequently result in unexpected output. Take for example, a user that looked at their asset alloc for years, and one day, an importer (or a plugin) adds one single price conversion statement. The asset alloc would drastically change, leaving the user either confused (in the best case), or having them not even notice, and make financial decisions based on an asset allocation that doesn't match their mental model.

In addition, it would be impossible to have a price conversion on a certain asset (for unconnected reasons), and still include it in the asset alloc or vice versa.

An explicit way of specifying configuration would thus be a basic requirement to have all this work correctly. For example, a config to explicitly list currencies to include and those to exclude, perhaps. That would have to be thought through.

[aldur]

I have just marked this PR as "draft" since it is obviously not ready for prime time :) If I have time, I'll try to give it more thought.

Continuing on your configuration note, what if instead users could explicitly opt-out assets from their allocation, e.g. as a metadata?

A-la Python: explicit is better than implicit :) That way, no price posting would change their allocation without their knowing, etc. As for me, that would allow me to just mark assets I do not have a price for as excluded and still get the rest of the allocation.

[redstreet]

Yes, configuration either through metadata or through the existing configuration dictionary would both be reasonable options to consider. The questions I'd ask are:

• is inclusion/exclusion based on currency a common enough use case with asset allocation, that it is valuable to code and maintain?
• wouldn't exclusion by account be the much more common case?
• if an account tree were structured as Assets:Investments:Bank1:USD, Assets:Investments:Bank1:EUR and such (which is a common structuring, and helpful for many reasons), couldn't they simply filter it out using the existing regex based config?

[aldur]

• is inclusion/exclusion based on currency a common enough use case with asset allocation, that it is valuable to code and maintain?
• wouldn't exclusion by account be the much more common case?

Maybe there was confusion based on my usage of the word "asset": I was thinking to "assets" as "accounts", rather than currency. In my case, I'd simply skip something like Assets:XYZ:ABC, etc.

• if an account tree were structured as Assets:Investments:Bank1:USD, Assets:Investments:Bank1:EUR and such (which is a common structuring, and helpful for many reasons), couldn't they simply filter it out using the existing regex based config?

I just tested this since I always prefer re-using existing functionality. My configuration was, previously, as follows:

2024-03-23 custom "fava-extension" "fava_investor" "{
  'asset_alloc_by_class' : {
      'accounts_patterns': ['Assets:.*'],
  }
}"

The accounts triggering the issue are of the form Assets:C1:AAA, Assets:C2:BBB, etc. So I resorted to negative lookahead:

2024-03-23 custom "fava-extension" "fava_investor" "{
  'asset_alloc_by_class' : {
      'accounts_patterns': ['Assets:(?!C1:AAA)(?!C2:BBB).*'],
  }
}"

This works. Now, is it better to specify the exclusion here or through a commodity metadata?

[redstreet]

That's exactly the way to configure it :).