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

Code before:

        self.amount = total_shares

    @expect_types(asset=Asset)
    def adjust_commission_cost_basis(self, asset, cost):
        """
        A note about cost-basis in zipline: all positions are considered
        to share a cost basis, even if they were executed in different
        transactions with different commission costs, different prices, etc.

        Due to limitations about how zipline handles positions, zipline will
        currently spread an externally-delivered commission charge across
        all shares in a position.
        """

        if asset != self.asset:
            raise Exception('Updating a commission for a different asset?')
        if cost == 0.0:
            return

        # If we no longer hold this position, there is no cost basis to
        # adjust.
        if self.amount == 0:
            return

        prev_cost = self.cost_basis * abs(self.amount)
        if isinstance(asset, Future):
            cost_to_use = cost / asset.multiplier
        else:
            cost_to_use = cost
        new_cost = prev_cost + cost_to_use
        self.cost_basis = new_cost / abs(self.amount)

    def __repr__(self):
        template = "asset: {asset}, amount: {amount}, cost_basis: {cost_basis}, \
last_sale_price: {last_sale_price}"
        return template.format(
            asset=self.asset,
            amount=self.amount,
            cost_basis=self.cost_basis,
            last_sale_price=self.last_sale_price
        )

    def to_dict(self):
        """
        Creates a dictionary representing the state of this position.
        Returns a dict object of the form:
        """
        return {
            'sid': self.asset,
            'amount': self.amount,

Review comment (bug):
@ssanderson and I had an in person chat were we discussed whether this was the correct change or not.

That conversation hinges on what we determine on what we intend `cost_basis` to mean. If it is meant to be a value to represent a price at which the the position would break even, the current implementation was correct. 

However, if cost basis is meant as the cost basis for tax reporting purposes, then I believe your change is correct. Since, during a short sale, the stocks are briefly held and then sold at current market price. I can definitely be wrong here, and we should verify, but in that case for tax reasons the cost basis would be from that sale of the shares.

(I checked `Position.update`, and there I do believe we are correctly handling the `cost_basis` for the second definition, when covering a short, by holding on to the `cost_basis` of the initial short sale until the short is fully covered.)

If that clarification blocks, we could separate this commit out from the per units fix.

Code after:

        self.amount = total_shares

    @expect_types(asset=Asset)
    def adjust_commission_cost_basis(self, asset, cost):
        """
        A note about cost-basis in zipline: all positions are considered
        to share a cost basis, even if they were executed in different
        transactions with different commission costs, different prices, etc.

        Due to limitations about how zipline handles positions, zipline will
        currently spread an externally-delivered commission charge across
        all shares in a position.
        """

        if asset != self.asset:
            raise Exception('Updating a commission for a different asset?')
        if cost == 0.0:
            return

        # If we no longer hold this position, there is no cost basis to
        # adjust.
        if self.amount == 0:
            return

        # We treat cost basis as the share price where we have broken even.
        # For longs, commissions cause a relatively straight forward increase
        # in the cost basis.
        #
        # For shorts, you actually want to decrease the cost basis because you
        # break even and earn a profit when the share price decreases.
        #
        # Shorts are represented as having a negative `amount`.
        #
        # The multiplication and division by `amount` cancel out leaving the
        # cost_basis positive, while subtracting the commission.

        prev_cost = self.cost_basis * self.amount
        if isinstance(asset, Future):
            cost_to_use = cost / asset.multiplier
        else:
            cost_to_use = cost
        new_cost = prev_cost + cost_to_use
        self.cost_basis = new_cost / self.amount

    def __repr__(self):
        template = "asset: {asset}, amount: {amount}, cost_basis: {cost_basis}, \
last_sale_price: {last_sale_price}"
        return template.format(
            asset=self.asset,
            amount=self.amount,