Repository: quantopian/zipline
File: zipline/finance/performance/position_tracker.py (Python)

Code before:
    def get_positions(self):

        positions = self._positions_store

        for sid, pos in iteritems(self.positions):

            if pos.amount == 0:
                # Clear out the position if it has become empty since the last
                # time get_positions was called.  Catching the KeyError is
                # faster than checking `if sid in positions`, and this can be
                # potentially called in a tight inner loop.
                try:
                    del positions[sid]
                except KeyError:
                    pass
                continue

            # Returns the position if we have one, or a default position if we
            # don't
            position = positions[sid]
            position.amount = pos.amount
            position.cost_basis = pos.cost_basis
            position.last_sale_price = pos.last_sale_price
            position.last_sale_date = pos.last_sale_date

            # Adds the new position if we didn't have one before
            positions[sid] = position

        return positions

    def get_positions_list(self):
        positions = []
        for sid, pos in iteritems(self.positions):
            if pos.amount != 0:
                positions.append(pos.to_dict())
        return positions

    def sync_last_sale_prices(self, dt, handle_non_market_minutes,
                              data_portal):
        if not handle_non_market_minutes:
            for asset, position in iteritems(self.positions):
                last_sale_price = data_portal.get_spot_value(
                    asset, 'price', dt, self.data_frequency
                )

                if not np.isnan(last_sale_price):
                    position.last_sale_price = last_sale_price
        else:
            for asset, position in iteritems(self.positions):
                last_sale_price = data_portal.get_adjusted_value(
                    asset,

Review comment (suggestion):
it's a bit confusing to be getting or creating the position via `positions[sid]`.  I think it would be clearer to explicitly create a new position on line 355, assign it all the properties, then put it into `positions` via the code on line 362.


Code after:
    def get_positions(self):

        positions = self._positions_store

        for sid, pos in iteritems(self.positions):

            if pos.amount == 0:
                # Clear out the position if it has become empty since the last
                # time get_positions was called.  Catching the KeyError is
                # faster than checking `if sid in positions`, and this can be
                # potentially called in a tight inner loop.
                try:
                    del positions[sid]
                except KeyError:
                    pass
                continue

            position = zp.Position(sid)
            position.amount = pos.amount
            position.cost_basis = pos.cost_basis
            position.last_sale_price = pos.last_sale_price
            position.last_sale_date = pos.last_sale_date

            # Adds the new position if we didn't have one before, or overwrite
            # one we have currently
            positions[sid] = position

        return positions

    def get_positions_list(self):
        positions = []
        for sid, pos in iteritems(self.positions):
            if pos.amount != 0:
                positions.append(pos.to_dict())
        return positions

    def sync_last_sale_prices(self, dt, handle_non_market_minutes,
                              data_portal):
        if not handle_non_market_minutes:
            for asset, position in iteritems(self.positions):
                last_sale_price = data_portal.get_spot_value(
                    asset, 'price', dt, self.data_frequency
                )

                if not np.isnan(last_sale_price):
                    position.last_sale_price = last_sale_price
        else:
            for asset, position in iteritems(self.positions):
                last_sale_price = data_portal.get_adjusted_value(
                    asset,
                    'price',