Closed, zeroed accounts using NONE booking show on balance sheet

[rsesek]

For accounts like 401k plans, until AVERAGE booking is implemented, we are advised to use NONE booking. My plan recently did a fund swap and after that transaction, despite the balance of the old account being 0 and being closed, it still shows on Fava's Balance Sheet display. I've attached a reduced booking-none-test.bean.txt file that demonstrates this.

I tracked the issue down to template_filter.should_show(), which uses account.balance_children.is_empty() to determine if there are any inventory still associated with the account. For accounts that use full booking, reductions will remove positions of an inventory and will cause the account to be empty when all are removed. But for NONE booked accounts, reductions are stored as negative positions, with the sum of the positions producing the net balance.

The issue thus appears to be in core.inventory.CounterInventory.is_empty(), since it does:

    def is_empty(self):
        """Check if the inventory is empty."""
        return not bool(self)

As a quick hack, I changed the implementation to:

    def is_empty(self):
        """Check if the inventory is empty."""
        return not bool(self) or sum([a for _, a in self.items()]) == 0

The above is probably not the right fix since I'm not an expert, but it did resolve the issue for me.

[yagebu]

Thanks for reporting this. We should fix this in the code of should_show (CounterInventory should have the same behaviour as Inventory in Beancount). I hope the performance implications aren't too bad.

[rmiceli]

I also have an investment account that shows in fava after all inventory was transferred out of the account. Changing is_empty as suggested above fixes the bug and does not have any noticeable performance implications.

However, when viewing the Balance Sheet at Market Value, the unrealized PnL is not carried over from the old hidden account and the unrealized PnL displayed only reflects the PnL since the account conversion. The total unrealized PnL for all assets accounts is still accurate, but the unrealized PnL from the old hidden account gets lost. Is that a beancount or fava issue?

[isTravis]

I'm also bumping into this. I'm curious if the isEmpty suggestion above is likely to make it into the codebase in the near future - or if it led to any performance issues in testing.

Thanks all - I was really scratching my head on this one until I found this Issue!