2
\$\begingroup\$

The idea of the code is to count how many elements in testy_L2 are the same with those on testy. So for example, in the first row of conf_matrix we have that 17 elements on testy_L2 are 0., one element is 1., and two elements are 2., so we have 17 correct numbers and 3 wrong numbers for 0. in testy_L2.

What do you think of this code? Is there another way to make it simpler/more elegant?

list_0 = []
a,b,c = 0,0,0
for j in indexes_0[0]:
    if testy_L2[j] == 0.0:
        a += 1
    if testy_L2[j] == 1.0:
        b += 1
    if testy_L2[j] == 2.0:
        c += 1
list_0.append(a)
list_0.append(b)
list_0.append(c)

list_1 = []
a,b,c = 0,0,0
for j in indexes_1[0]:
    if testy_L2[j] == 0.0:
        a += 1
    if testy_L2[j] == 1.0:
        b += 1
    if testy_L2[j] == 2.0:
        c += 1
list_1.append(a)
list_1.append(b)
list_1.append(c) 

list_2 = []
a,b,c = 0,0,0
for j in indexes_2[0]:
    if testy_L2[j] == 0.0:
        a += 1
    if testy_L2[j] == 1.0:
        b += 1
    if testy_L2[j] == 2.0:
        c += 1
list_2.append(a)
list_2.append(b)
list_2.append(c)

Inputs:

testy:

array([1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1.,
       1., 1., 1., 2., 2., 2., 2., 2., 2., 2., 2., 2., 2., 2., 2., 2., 2.,
       2., 2., 2., 2., 2., 2., 2., 2., 0., 0., 0., 0., 0., 0., 0., 0., 0.,
       0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.])

testy_L2:

array([0., 0., 0., 1., 1., 0., 1., 0., 0., 1., 0., 1., 0., 0., 1., 1., 1.,
       0., 1., 1., 2., 2., 2., 2., 2., 2., 2., 2., 2., 2., 2., 2., 2., 2.,
       2., 2., 2., 2., 2., 2., 2., 2., 1., 0., 0., 0., 0., 0., 0., 0., 0.,
       0., 0., 0., 2., 0., 2., 0., 0., 0., 0., 0.])

Indexes_0, Indexes_1, Indexes_2:

for i in range(len(testy)):
    indexes_0, indexes_1, indexes_2 = np.where(testy == 0.0), np.where(testy == 1.0), np.where(testy == 2.0)

Output:

conf_matrix:

conf_matrix = np.array([list_0,list_1,list_2])

array([[17,  1,  2],
       [10, 10,  0],
       [ 0,  0, 22]])
\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Code Review! The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How to Ask for examples, and revise the title accordingly. \$\endgroup\$ Jan 7 '21 at 21:12
4
\$\begingroup\$

You have essentially one chunk of code that's duplicated three times:

list_N = []
a,b,c = 0,0,0
for j in indexes_N[0]:
    if testy_L2[j] == 0.0:
        a += 1
    if testy_L2[j] == 1.0:
        b += 1
    if testy_L2[j] == 2.0:
        c += 1
list_N.append(a)
list_N.append(b)
list_N.append(c)

This could be easily wrapped in a function:

def process_the_stuff(indexes):
    acc = []
    a,b,c = 0,0,0
    for j in indexes[0]:
        if testy_L2[j] == 0.0:
            a += 1
        if testy_L2[j] == 1.0:
            b += 1
        if testy_L2[j] == 2.0:
            c += 1
    acc.append(a)
    acc.append(b)
    acc.append(c)
    return acc

Then just:

list_0 = process_the_stuff(indexes_0)
list_1 = process_the_stuff(indexes_1)
list_2 = process_the_stuff(indexes_2)

I'd clean up some other stuff though:

  • I'm not a fan of assigning multiple variables on a line in the vast majority of cases. I think a, b, and c should be initially assigned on separate lines.
  • The three append calls at the bottom could be cleaned up a bit by using extend, or just simply returning a tuple/list directly.
  • All three if checks are necessarily exclusive of each other (since testy_L2[j] can't be equal to 0, 1, and 2 at the same time). Those should be elifs instead so that once one is true, the others aren't checked.

After fixing these things, you'd be left with:

def process_the_stuff(indexes):
    a = 0
    b = 0
    c = 0
    for j in indexes[0]:
        if testy_L2[j] == 0.0:
            a += 1
        elif testy_L2[j] == 1.0:
            b += 1
        elif testy_L2[j] == 2.0:
            c += 1
    return [a, b, c]  # May make more sense to return a tuple instead unless you need a list

list_0 = process_the_stuff(indexes_0)
list_1 = process_the_stuff(indexes_1)
list_2 = process_the_stuff(indexes_2)

Minor, but "indexes" should be "indices" for the plural of "index".

\$\endgroup\$
1
  • \$\begingroup\$ Thank you! Really helpful. And sorry for the 'indexes' for I'm not native english speaker. \$\endgroup\$
    – immb31
    Jan 7 '21 at 21:54
1
\$\begingroup\$

Here's code from a well tested library that accomplishes the same thing.

Documentation

from sklearn.metrics import confusion_matrix
conf_matrix = confusion_matrix(testy,testy_L2)

Inline Implementation

Without more imports, this can still be achieved with very little code.

def confusion_matrix(Y_true, Y_predicted):
    matrix = np.zeros((3,3),dtype=int)
    for y_true,y_predicted in zip(
        Y_true.astype('int'), 
        Y_predicted.astype('int')
    ):
        matrix[y_true,y_predicted] += 1
    return matrix
\$\endgroup\$
1
  • \$\begingroup\$ Thank you, but the idea was to make up with a code to it. \$\endgroup\$
    – immb31
    Jan 9 '21 at 15:32

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.