Stock trading script

Question

The script uses a financial indicator called moving averages and their crossover points to recommend best actions for stock trading (buy vs sell).

• starting_cash: initial cash position
• prices: list of prices (ordered by time)
• crossovers: list of crossover points on the prices, crossovers[i] is a list [time_index, buy_index]
• buy_index = 1 means "buy" and buy_index = 2 means "sell"

I'm just looking to simplify the script but can provide more context if anyone wants it.

def make_trades(starting_cash, prices, crossovers):

    value = [starting_cash] #set value to starting_cash
    value = []  # creating empty set?
    stock_at = 0
    cash = starting_cash

    for i in range(0,len(prices)):
        added_value = (prices[i] - prices[i-1])/prices[i-1]
        try:
            time_index = crossovers[0][0]
            buy_index = crossovers[0][1]              
            if i == time_index:            
                if buy_index == 1:
                    stock_at = cash
                    cash = 0 
                    value.append(stock_at)                                                           
                    crossovers = crossovers[1:]
                else:
                    if(stock_at == 0):
                        cash = value[i-1]
                        value.append(value[i-1])
                    else:
                        cash = value[i-1] + value[i-1]*added_value
                        stock_at = 0
                        value.append(cash)
                    crossovers = crossovers[1:]
            else:
                if cash == 0:                
                    stock_at = value[i-1] + value[i-1]*added_value
                    value.append(stock_at)
                else:
                    value.append(cash)                      
        except (IndexError):
            added_value = (prices[i] - prices[i-1])/prices[i-1]
            if cash == 0:
                stock_at = value[i-1] + value[i-1]*added_value
                value.append(stock_at)
            else:
                value.append(cash)            
    return [round(i, 2) for i in value]

Answer 1

In this code:

for i in range(0,len(prices)):
    added_value = (prices[i] - prices[i-1])/prices[i-1]

You start with i = 0. In that case, prices[i-1] will be prices[-1] which will access the last element in the prices list. That is, prices[len(prices) - 1].

I think this probably isn't what you want. You might want to do:

for i in range(1, len(prices)):

instead, so that prices[i-1] starts at zero. But then you won't run the loop on i=0, if that matters.

Alternatively, you might want to try:

last_price = 0
for i, price in enumerate(prices):
    added_value = 1.00 if last_price == 0 else (price - last_price)/last_price

    # ...
    last_price = price

Get rid of magic numbers

Your declaration that "buy_index == 1" means buy, and 2 means sell is ridiculous. If your code produces those numbers, then change your code. If you are reading the numbers from somewhere, change the numbers:

crossovers = ((t, 'buy' if a == 1 else 'sell') for t,a in crossovers)

You might use an enum instead, but if you're going to JSON, or reading from JSON, the string is probably easier.

Use modern Python idioms

One of the tenets of "modern Python" (3.x-style) is a bias towards iteration, especially the notion of infinite iterables. In other words:

1. Expect iterables, not sequences.
2. Generate, don't return a list.

I suggest that you convert prices to a series of (time, price) tuples, so that there is only one "source of truth" when it comes to what time should associate with a price.

Next, treat prices and crossovers as series, not lists: assume they are potentially infinite, and iterate them instead of trying to index them:

xo = iter(crossovers)
x_time, x_action = next(xo)

for time, price in prices:
    if time == x_time:
        if x_action == 'buy':
            #...
        x_time, x_action = next(xo)

Similarly, don't build a values = [] list and return it. Instead, yield your values when appropriate. And since they aren't indexed in a list, provide a timestamp to go along:

        yield time, round(cash, 2)
        # or
        yield time, round(stock_at, 2)