10
\$\begingroup\$

What I am trying to accomplish is calculate the Tribonacci numbers and the ratio. I can do this in Excel easy like this.

Image 1

Image 2

So I tried do the same using Python 3 now. It works and gives such output.

Image 3

But would you be so kind to review the code and see what obvious mistakes you see in it and advise on what is a better approach as I do not want to continue coding if my logic to tackle such problem is fundamentally flawed somewhere.

Priorities for me are execution time and readability I guess

What would you have done differently and why? I know it may look like a mess. From what I heard using exec and eval (code as string) are the bad ideas and also that function calls are expensive?

I don't like my code at all especially how next() is used so many times but hopefully you can see how I was thinking trying to replicate the Excel spreadsheet

To run the code you need numpy and pandas

import pandas as pd
import numpy as np

column_names = ["Number sequence", "Ratio"]
column_counter = len(column_names) -1
header_columns = ['']
while column_counter >= 0:
    column_counter = column_counter - 1
    header_columns.append(column_names[column_counter])

tribonacci_numbers = [0,1]    
tribonacci_counter = 19 #Reach
while tribonacci_counter -1 >= 0:
    tribonacci_counter = tribonacci_counter - 1
    tribonacci_numbers.append(sum(tribonacci_numbers[-3:]))
tribonaccis_list_length = len(tribonacci_numbers)-1

index_builder = 1
rows = []
while index_builder <= tribonaccis_list_length:
    try:
        index_builder = index_builder + 1
        rows.append((["Number: "+str(index_builder),tribonacci_numbers[index_builder],tribonacci_numbers[index_builder]/tribonacci_numbers[index_builder-1]]))       
    except IndexError:
        continue

def get_row():
    the_row = [x for x in rows]
    row_counter = 0
    while row_counter <= len(the_row):
        try:
            row_counter = row_counter + 1
            yield (the_row[row_counter])            
        except IndexError:
            continue

a = get_row()
def therow():
    for q in a:
        yield q

datas = np.array([
header_columns,
next(therow()),
next(therow()),
next(therow()),
next(therow()),
next(therow()),
next(therow()),
next(therow()),
next(therow()),
next(therow()),
next(therow()),
next(therow()),
next(therow()),
next(therow()),
next(therow()),
next(therow()),
next(therow()),
next(therow()),
next(therow())])

df = pd.DataFrame(data=datas[1:,1:],index=datas[1:,0],columns=datas[0,1:])
print(df)

If you want to provide an example that does the same and produces a Pandas dataframe like this it would be very useful for me to study your code.

\$\endgroup\$
  • 4
    \$\begingroup\$ a tip, instead of posting screenshots of the expected result, give it as a copy-pasteable piece of text or code, like csv or json \$\endgroup\$ – Maarten Fabré Jan 29 '18 at 17:07
11
\$\begingroup\$

You’re using the wrong tool for the job. Basically, you do all the computation in Python, use numpy for intermediate storage and pandas for display.

Instead, you should compute the list of tribonacci numbers and from there on use pandas for anything else as it would be much more efficient / readable. I’d keep building the tribonacci numbers in Python as I don't know enough pandas to be able to come up with an efficient way that wouldn't involve Dataframe.append and also because it can be expressed very nicely as a generator:

def tribonacci_sequence():
    a, b, c = 0, 1, 1
    yield a
    yield b
    while True:
        yield c
        a, b, c = b, c, a + b + c

You can then select the amount of numbers you like using itertools.islice and give them to a pandas.Dataframe.

From there on, you can easily compute the ratio by shifting the dataframe and dividing element-wise:

def compute_tribonacci(upper_limit, skip_first_rows=3):
    tribonacci = list(islice(tribonacci_sequence(), upper_limit + 1))
    df = pd.DataFrame({'Number sequence': tribonacci})
    df['Ratio'] = df['Number sequence'] / df.shift(1)['Number sequence']
    df.index = df.index.map('Number: {}'.format)
    return df.iloc[skip_first_rows:]

Last thing to do is to actually ask for a certain amount of rows. Full code:

from itertools import islice
import pandas as pd


def tribonacci_sequence():
    a, b, c = 0, 1, 1
    yield a
    yield b
    while True:
        yield c
        a, b, c = b, c, a + b + c


def compute_tribonacci(upper_limit, skip_first_rows=3):
    tribonacci = list(islice(tribonacci_sequence(), upper_limit + 1))
    df = pd.DataFrame({'Number sequence': tribonacci})
    df['Ratio'] = df['Number sequence'] / df.shift(1)['Number sequence']
    df.index = df.index.map('Number: {}'.format)
    return df.iloc[skip_first_rows:]


if __name__ == '__main__':
    df = compute_tribonacci(50)
    print(df)
\$\endgroup\$
  • \$\begingroup\$ "Basically, you do all the computation in Python" The simple present is used both to express what a person has done and what they should do (e.g. "When you get tho the hill, you turn right"). I recommend using past tense or present progressive to express things that a person has done to avoid that ambiguity. "it can be expressed very nicely as a generator" Except your method reassigns variables every time it's called. With a list, you append each number and then that's it. \$\endgroup\$ – Acccumulation Jan 29 '18 at 22:43
  • \$\begingroup\$ Thanks for the grammar lesson but this is a code review site :) \$\endgroup\$ – яүυк Jan 29 '18 at 23:00
  • 1
    \$\begingroup\$ Thank you for your responses and answers, much appreciated. I explored and understood your codes. I will likely use code review more in the future hopefully improving each time. One thing I am concerned about is when reading "What’s New In Python 3.7" here docs.python.org/3.7/whatsnew/3.7.html I see that "Yield expressions (both yield and yield from clauses) are now deprecated in comprehensions and generator expressions..." I guess it might stop working after updating python and generate a SyntaxError in the future...? \$\endgroup\$ – Cactus Jan 29 '18 at 23:48
  • \$\begingroup\$ @Cactus you should not worry about 3.7; we are far from it, besides that you can use shebang. \$\endgroup\$ – Kyslik Jan 30 '18 at 0:56
  • \$\begingroup\$ @Cactus The yield keyword has always been and will always be used to define a generator. I don't use generator expressions or list comprehension in this code, let alone using yield with these forms. \$\endgroup\$ – Mathias Ettinger Jan 30 '18 at 7:17
6
\$\begingroup\$

Some general tips

Put code in functions

That way you can test each part individually. Here the generation of the sequence, calculation of the ratio and exporting to pandas are clear divisions in the functionality of your code

Generators

The best algorithms implement fibonacci sequences as generators

with limit

def fib(n):
    a, b = 0, 1
    for _ in range(n):
        yield a
        a, b = b, a + b

or endless

def fib():
    a, b = 0, 1
    while True:
        yield a
        a, b = b, a + b

Adapting this to tribonacci should be trivial

def trib():
    a, b, c = 0, 1, 1
    while True:
        yield a
        a, b, c = b, c, a + b + c

or more generalized:

def generalised_fibonacci(max_len = 2, begin_data = None):
    from collections import deque
    if begin_data is None:
        begin_data = [0] + [1] * (max_len-1)
    data = deque(begin_data, max_len)
    while True:
        # print(data)
        a = data.popleft()
        yield a
        data.append(a + sum(data))

optimised to @Accumulation's remark

def generalised_fibonacci(max_len = 2, begin_data = None):
    from collections import deque
    if begin_data is None:
        begin_data = [0] + [1] * (max_len - 1) + [max_len - 1]
    data = deque(begin_data, max_len + 1)
    while True:
        a = data.popleft()
        yield a
        # print(a, data)
        data.append(2 * data[-1] - a)

Get it into pandas

here list and itertools.islice are your friends

def get_trib(num_items)
    trib_nums = list(itertools.islice(trib(3), num_items + 1))
    return pd.DataFrame(data=trib_nums, index = range(num_items))

calculate the ratios

pandas has a simple DataFrame.shift method, but it is also possible to calculate it before the introduction into the DataFrame

def trib_ratio(num_items):

    trib_generator = itertools.islice(generalised_fibonacci(3), num_items + 1)
    trib_gen, trib_gen2 = itertools.tee(trib_generator, 2)
    yield next(trib_gen), None

    for trib, trib2 in zip(trib_gen, trib_gen2):
        ratio = trib / trib2 if trib2 else None  # prevent zerodivision on first element
        yield trib, ratio
pd.DataFrame(list(trib_ratio(20)), columns=['trib', 'ratio'])
    trib    ratio
0   0   
1   1   
2   1   1.0
3   2   2.0
4   4   2.0
5   7   1.75
6   13  1.8571428571428572
7   24  1.8461538461538463
8   44  1.8333333333333333
9   81  1.8409090909090908
10  149 1.8395061728395061
11  274 1.8389261744966443
12  504 1.8394160583941606
13  927 1.8392857142857142
14  1705    1.8392664509169363
15  3136    1.8392961876832845
16  5768    1.8392857142857142
17  10609   1.8392857142857142
18  19513   1.8392873974926949
19  35890   1.8392866294265362
20  66012   1.8392867093898022

This can be generalized for more and further ratios, by adapting the tee and keeping the different generators in a data structure

\$\endgroup\$
  • \$\begingroup\$ +1 for generalized_fibonacci \$\endgroup\$ – Austin Hastings Jan 29 '18 at 19:00
  • 2
    \$\begingroup\$ For the generalized Fibonacci case, it's more efficient to popleft, then subtract that from twice the last number (you can check with the tribonacci numbers that f(n+1) = 2*f(n) - f(n-3); in general, f(n+1) = 2*f(n)-f(n-k) for kbonacci numbers). \$\endgroup\$ – Acccumulation Jan 29 '18 at 22:55
  • \$\begingroup\$ Thank you for the tips. The generalized version is indeed nice. Do you think it is possible to make a version which returns negative ratios as well? For example "-0.618" which can be calculated as 0.382 - 1 0.382 is from dividing 55/144. It is also found by subtracting 0.618 from 1 (1 – 0.618 = 0.328) and by finding the square of 0.618 (0.618 x 0.618 = 0.382). \$\endgroup\$ – Cactus Jan 29 '18 at 23:45
  • \$\begingroup\$ you can calculate any ratio you want \$\endgroup\$ – Maarten Fabré Jan 30 '18 at 8:47
3
\$\begingroup\$

Create a dataframe with the given column names:

df = pd.DataFrame(columns=['Number sequence', 'Ratio'])

Build each row:

for row_number in range(4,number_of_rows):
     row_name = 'Number: {}'.format(row_number)
     dropped_number = df.loc['Number: {}'.format(row_number-4),'Number Sequence']
     current_number = 2*previous_number-dropped_number
     df.loc[row_name,'Number Sequence'] = current_number
     df.loc[row_name,'Ratio'] = current_number/previous_number
     previous_number = current_number

if you're having trouble understanding current_number = 2*previous_number-last_number, consider the case of generating the 7th number. This is going to be 4+7+13. But 13 was calculated by 13 = 2+4+7. So the 7th number is 4+7+(2+4+7) = 2*(2+4+7)-2, which is 2*(6th number) - 3rd number. So the nth number is 2*(n-1)th number - (n-4)th number.

This does require handling the first few rows separately. One way is to manually create them, and then if you don't want them, you can delete them afterwards. Note that if you're deleting them, then you don't have to fill in the ratio column for those rows.

You will also have to initialize previous_number to the proper value.

\$\endgroup\$
  • \$\begingroup\$ Thank you. From the answers provided I learnt Numpy is not needed at all for this task as a separate import and I can see how your solution is efficient by reusing the previous number this way. \$\endgroup\$ – Cactus Jan 30 '18 at 0:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.