Repository: beancount/fava
Issue: Net worth disagrees with bean-report

When I run bean-report networth I get different numbers in my two currencies than from fava.  The bean-report numbers are higher and closer to what I was seeing about a month ago with fava.  My guess is something changed.


Changed files:
fava/api/__init__.py, fava/application.py, fava/api/helpers.py

diff --git a/fava/api/__init__.py b/fava/api/__init__.py
index 222954cc..28bde30c 100644
--- a/fava/api/__init__.py
+++ b/fava/api/__init__.py
@@ -29,8 +29,9 @@ from beancount.core.realization import RealAccount
 from beancount.core.interpolate import compute_entries_balance
 from beancount.core.account import has_component
 from beancount.core.account_types import get_account_sign
-from beancount.core.data import get_entry, posting_sortkey, Close, Note,\\
-                                Document, Balance, Transaction, Event, Query
+from beancount.core.data import (get_entry, iter_entry_dates, posting_sortkey,
+                                 Close, Note, Document, Balance, Transaction,
+                                 Event, Query)
 from beancount.core.number import ZERO
 from beancount.ops import prices, holdings, summarize
 from beancount.parser import options
@@ -39,11 +40,21 @@ from beancount.reports import context
 from beancount.utils import misc_utils
 
 from fava.util.dateparser import parse_date
-from fava.api.helpers import entries_in_inclusive_range,\\
-                                      holdings_at_dates
+from fava.api.helpers import holdings_at_dates
 from fava.api.serialization import serialize_inventory, serialize_entry
 
 
+def get_next_interval(date_, interval):
+    if interval == 'year':
+        return date(date_.year + 1, 1, 1)
+    elif interval == 'month':
+        month = (date_.month % 12) + 1
+        year = date_.year + (date_.month + 1 > 12)
+        return date(year, month, 1)
+    else:
+        raise NotImplementedError
+
+
 class FilterException(Exception):
     pass
 
@@ -200,23 +211,11 @@ class BeancountReportAPI(object):
         if not date_first:
             return []
 
-        def get_next_interval(date_, interval):
-            if interval == 'year':
-                return date(date_.year + 1, 1, 1)
-            elif interval == 'month':
-                month = (date_.month % 12) + 1
-                year = date_.year + (date_.month + 1 > 12)
-                return date(year, month, 1)
-            else:
-                raise NotImplementedError
-
-        date_first = date(date_first.year, date_first.month, 1)
-        date_last = get_next_interval(date_last, interval) - timedelta(days=1)
-
         interval_tuples = []
         while date_first <= date_last:
-            interval_tuples.append((date_first, get_next_interval(date_first, interval) - timedelta(days=1)))
-            date_first = get_next_interval(date_first, interval)
+            next_date = get_next_interval(date_first, interval)
+            interval_tuples.append((date_first, next_date))
+            date_first = next_date
 
         return interval_tuples
 
@@ -229,14 +228,15 @@ class BeancountReportAPI(object):
         names = [account_name] if isinstance(account_name, str) else account_name
 
         interval_tuples = self._interval_tuples(interval, self.entries)
-        date_first, date_last = getters.get_min_max_dates(self.entries, (Transaction))
+        date_first, _ = getters.get_min_max_dates(self.entries, (Transaction))
         return [{
             'begin_date': begin_date,
             'end_date': end_date,
             'totals': self._balances_totals(names, begin_date if not accumulate else date_first, end_date),
         } for begin_date, end_date in interval_tuples]
 
-    def _real_account(self, account_name, entries, begin_date=None, end_date=None, min_accounts=None):
+    def _real_account(self, account_name, entries, begin_date=None,
+                      end_date=None, min_accounts=None):
         """
         Returns the realization.RealAccount instances for account_name, and
         their entries clamped by the optional begin_date and end_date.
@@ -246,13 +246,13 @@ class BeancountReportAPI(object):
 
         :return: realization.RealAccount instances
         """
-        entries_in_range = entries_in_inclusive_range(entries, begin_date=begin_date, end_date=end_date)
+        if begin_date:
+            entries = list(iter_entry_dates(entries, begin_date, end_date))
         if not min_accounts:
             min_accounts = [account_name]
-        real_account = realization.get(realization.realize(entries_in_range, min_accounts), account_name)
-
-        return real_account
 
+        return realization.get(realization.realize(entries, min_accounts),
+                               account_name)
 
     def balances(self, account_name, begin_date=None, end_date=None, min_accounts=None):
         """
@@ -361,20 +361,30 @@ class BeancountReportAPI(object):
     def _net_worth_in_periods(self):
         month_tuples = self._interval_tuples('month', self.entries)
         monthly_totals = []
-        end_dates = [p[1] + timedelta(days=1) for p in month_tuples]
-
-        for (begin_date, end_date), holdings_list in zip(month_tuples,
-                                                        holdings_at_dates(entries=self.entries,
-                                                                          dates=end_dates,
-                                                                          options_map=self.options,
-                                                                          price_map=self.price_map)):
-            totals = dict()
+        end_dates = [p[1] for p in month_tuples]
+
+        for (begin_date, end_date), holdings_list in \\
+                zip(month_tuples, holdings_at_dates(self.entries, end_dates,
+                                                    self.price_map, self.options)):
+            totals = {}
             for currency in self.options['operating_currency']:
-                total = ZERO
-                for holding in holdings.convert_to_currency(self.price_map, currency, holdings_list):
-                    if holding.cost_currency == currency and holding.market_value:
-                        total += holding.market_value
-                totals[currency] = total
+                currency_holdings_list = \\
+                    holdings.convert_to_currency(self.price_map, currency,
+                                                 holdings_list)
+                if not currency_holdings_list:
+                    continue
+
+                holdings_list = holdings.aggregate_holdings_by(
+                    currency_holdings_list, operator.attrgetter('cost_currency'))
+
+                holdings_list = [holding
+                                 for holding in holdings_list
+                                 if holding.currency and holding.cost_currency]
+
+                # If after conversion there are no valid holdings, skip the currency
+                # altogether.
+                if holdings_list:
+                    totals[currency] = holdings_list[0].market_value
 
             monthly_totals.append({
                 'begin_date': begin_date,
diff --git a/fava/api/helpers.py b/fava/api/helpers.py
index 1ffc1416..8cdf75cb 100644
--- a/fava/api/helpers.py
+++ b/fava/api/helpers.py
@@ -1,32 +1,9 @@
-from datetime import timedelta
-import re
-
-from beancount.core import flags
+from beancount.core import flags, account_types
 from beancount.ops.holdings import Holding
 from beancount.core.data import Transaction
 from beancount.core.inventory import Inventory
 from beancount.parser import options
 from beancount.ops import prices
-from beancount.utils import bisect_key
-
-
-def entries_in_inclusive_range(entries, begin_date=None, end_date=None):
-    """
-    Returns the list of entries satisfying begin_date <= date <= end_date.
-    """
-    get_date = lambda x: x.date
-    if begin_date is None:
-        begin_index = 0
-    else:
-        begin_index = bisect_key.bisect_left_with_key(entries, begin_date,
-                                                      key=get_date)
-    if end_date is None:
-        end_index = len(entries)
-    else:
-        end_index = bisect_key.bisect_left_with_key(entries,
-                                                    end_date+timedelta(days=1),
-                                                    key=get_date)
-    return entries[begin_index:end_index]
 
 
 # This really belongs in beancount:src/python/beancount/ops/holdings.py
@@ -48,7 +25,7 @@ def get_holding_from_position(position, price_map=None, date=None):
             price_date, price_number = prices.get_price(price_map,
                                                         base_quote, date)
             if price_number is not None:
-                market_value = number * price_number
+                market_value = position.units.number * price_number
         else:
             price_date, price_number = None, None
 
@@ -73,51 +50,35 @@ def get_holding_from_position(position, price_map=None, date=None):
                        None)
 
 
-def inventory_at_dates(entries, dates, transaction_predicate,
-                       posting_predicate):
+def inventory_at_dates(transactions, dates, posting_predicate):
     """Generator that yields the aggregate inventory at the specified dates.
 
     The inventory for a specified date includes all matching postings PRIOR to
     it.
 
-    :param entries: list of entries, sorted by date.
+    :param transactions: list of transactions, sorted by date.
     :param dates: iterator of dates
-    :param transaction_predicate: predicate called on each Transaction entry to
-        decide whether to include its postings in the inventory.
     :param posting_predicate: predicate with the Transaction and Posting to
         decide whether to include the posting in the inventory.
     """
-    entry_i = 0
-    num_entries = len(entries)
+    index = 0
+    length = len(transactions)
 
     # inventory maps lot to amount
     inventory = Inventory()
     prev_date = None
     for date in dates:
-        assert prev_date is None or date >= prev_date
+        assert prev_date is None or date > prev_date
         prev_date = date
-        while entry_i < num_entries and entries[entry_i].date < date:
-            entry = entries[entry_i]
-            entry_i += 1
-            if isinstance(entry, Transaction) and transaction_predicate(entry):
-                for posting in entry.postings:
-                    if posting_predicate(entry, posting):
-                        inventory.add_position(posting)
+        while index < length and transactions[index].date < date:
+            entry = transactions[index]
+            index += 1
+            for posting in entry.postings:
+                if posting_predicate(posting):
+                    inventory.add_position(posting)
         yield inventory
 
 
-def account_descendants_re_pattern(*roots):
-    """Returns pattern for matching descendant accounts.
-
-    :param roots: The list of parent account names.  These should not
-        end with a ':'.
-
-    :return: The regular expression pattern for matching descendants of
-             the specified parents, or those parents themselves.
-    """
-    return '|'.join('(?:^' + re.escape(name) + '(?::|$))' for name in roots)
-
-
 def holdings_at_dates(entries, dates, price_map, options_map):
     """Computes aggregate holdings at mulitple dates.
 
@@ -130,17 +91,19 @@ def holdings_at_dates(entries, dates, price_map, options_map):
     :param price_map: A dict of prices, as built by prices.build_price_map().
     :param options_map: The account options.
     """
-    account_types = options.get_account_types(options_map)
-    FLAG_UNREALIZED = flags.FLAG_UNREALIZED
-    transaction_predicate = lambda e: e.flag != FLAG_UNREALIZED
-    account_re = re.compile(account_descendants_re_pattern(
-        account_types.assets,
-        account_types.liabilities))
-    posting_predicate = lambda e, p: account_re.match(p.account)
+    transactions = [entry for entry in entries
+                    if (isinstance(entry, Transaction) and
+                        entry.flag != flags.FLAG_UNREALIZED)]
+
+    types = options.get_account_types(options_map)
+
+    def posting_predicate(posting):
+        account_type = account_types.get_account_type(posting.account)
+        if account_type in (types.assets, types.liabilities):
+            return True
+
     for date, inventory in zip(dates,
-                               inventory_at_dates(
-                                   entries, dates,
-                                   transaction_predicate = transaction_predicate,
-                                   posting_predicate = posting_predicate)):
-        yield [get_holding_from_position(position, price_map=price_map, date=date)
-               for position in inventory.units()]
+                               inventory_at_dates(transactions, dates,
+                                                  posting_predicate)):
+        yield [get_holding_from_position(position, price_map, date)
+               for position in inventory]
diff --git a/fava/application.py b/fava/application.py
index cd75fb51..af53a7fd 100644
--- a/fava/application.py
+++ b/fava/application.py
@@ -20,21 +20,24 @@ app.secret_key = '1234'
 app.api = BeancountReportAPI()
 
 app.config.raw = configparser.ConfigParser()
-config_defaults_file = os.path.join(os.path.dirname(os.path.realpath(__file__)),
+defaults_file = os.path.join(os.path.dirname(os.path.realpath(__file__)),
                              'default-settings.conf')
-app.config_file = config_defaults_file
-app.config.raw.read(config_defaults_file)
+app.config_file = defaults_file
+app.config.raw.read(defaults_file)
 app.config.user = app.config.raw['fava']
-app.config.user['file_defaults'] = config_defaults_file
+app.config.user['file_defaults'] = defaults_file
 app.config.user['file_user'] = ''
 
+
 def load_user_settings(settings_file_path):
     app.config.user['file_user'] = settings_file_path
     app.config.raw.read(app.config.user['file_user'])
 
-    app.config_file = app.config.user['file_defaults'] \\
-                      if app.config.user['file_user'] == '' \\
-                      else app.config.user['file_user']
+    if app.config.user['file_user'] == '':
+        app.config_file = app.config.user['file_defaults']
+    else:
+        app.config_file = app.config.user['file_user']
+
 
 @app.route('/account/<name>/')
 def account_with_journal(name=None):
@@ -65,7 +68,8 @@ def document():
     if document_path and app.api.is_valid_document(document_path):
         # metadata-statement-paths may be relative to the beancount-file
         if not os.path.isabs(document_path):
-            document_path = os.path.join(os.path.dirname(os.path.realpath(app.beancount_file)), document_path)
+            document_path = os.path.join(os.path.dirname(
+                os.path.realpath(app.beancount_file)), document_path)
 
         directory = os.path.dirname(document_path)
         filename = os.path.basename(document_path)
@@ -134,7 +138,7 @@ def source():
         source = request.form['source']
 
         if file_path == app.config_file:
-            if file_path != config_defaults_file:
+            if file_path != defaults_file:
                 with open(file_path, 'w+', encoding='utf8') as f:
                     f.write(source)
             successful = True