Funktion für Presicion schreiben

[haa]

Guten Tag,

wir sollen für die Uni den Wert von Precision und Recall ermitteln und mit vorgegebenen Doctests testen.
Mein Problem ist, dass der erste Test in der for-Schleife das zweite if-Statement ignoriert und der Wert für Precision deshalb 1 ist. Beim zweiten Test kommt aber das richtige Ergebnis raus.
Der Fehler liegt ja vermutlich in dem zweiten if-statement, aber ich finde ihn einfach nicht.

Vielen Dank im Voraus

Code: Alles auswählen

import random

hams = "reminder deadline meet thanks thanks for tip deadline approaching".split(" ")
spams = "hot stock tip meet hot single".split(" ")
list1 = [('ham', 'ham')] * 58 + [('spam', 'ham')] * 21 + [('ham', 'spam')] * 9 + [('spam', 'spam')] * 12
random.shuffle(list1)


def precision(k, classes):
    """ Calculate the precision of class `k` for the classifications in `classes`.
    Each element in classes is a tuple.  The first element in the tuple specifies
    the classification of the classifier.  The second element specifies the real
    class. [2 points]
    >>> round(precision('spam', list1), 4)
    0.3636
    >>> round(precision('ham', list1), 4)
    0.8657
    >>> round(precision('spam', []), 4)
    0.0
    """
    tp = 0
    fp = 0
    p = ""

    if (k == "spam "):
        p = "ham"
    elif (k == "ham"):
        p = "spam"

    for i in range(len(classes)):
        if classes[i] == (k, k):
            tp += 1
        if classes[i] == (k, p):
            fp += 1

    if classes == []:
        return 0.0

    return tp / (tp + fp)

[__blackjack__]

@haa: Ich vermute mal eher Du möchtest *vor* der Schleife mal überprüfen ob `p` den Wert hat den es haben soll. Darf `p` da die leere Zeichenkette sein? Falls nein, sollte die Zeile ``p = ""`` besser überhaupt nicht existieren. Falls `p` eine leere Zeichenkette sein darf, würde man das besser in einen ``else``-Zweig schreiben statt davor, dann ist das verständlicher.

Sonstiges: ``for in in range(sequence):`` nur um `i` als Index in `sequence` zu verwenden ist in Python ein „anti-pattern“. Man kann direkt über die Elemente von Sequenzwerten iterieren, ohne den unnötigen Umweg über einen Laufindex.

`hams`, `spams`, und `list1` werden nirgends verwendet und sollten so auch gar nicht auf Modulebene existieren. Es sei denn es sind Konstanten, dann müssten sie aber KOMPLETT_GROSS geschrieben werden.

Grundatentypen haben in Namen nichts zu suchen und nummeriert werden Namen auch nicht.

Das zweite ``if`` in der ``for``-Schleife sollte wohl auch eher ein ``elif`` sein.

Warum `tp` *und* `fp` wenn die beiden Werte einzeln gar nicht gebraucht werden, sondern nur eine Gesamtzahl?

Zwischenstand:

Code: Alles auswählen

#!/usr/bin/env python3
import random

_TEST_CLASSES = (
    [("ham", "ham")] * 58
    + [("spam", "ham")] * 21
    + [("ham", "spam")] * 9
    + [("spam", "spam")] * 12
)
random.shuffle(_TEST_CLASSES)


def precision(k, classes):
    """
    Calculate the precision of class `k` for the classifications in `classes`.

    Each element in classes is a tuple.  The first element in the tuple
    specifies the classification of the classifier.  The second element
    specifies the real class. [2 points]

    >>> round(precision("spam", _TEST_CLASSES), 4)
    0.3636
    >>> round(precision("ham", _TEST_CLASSES), 4)
    0.8657
    >>> round(precision("spam", []), 4)
    0.0
    """
    if k == "spam ":
        p = "ham"
    elif k == "ham":
        p = "spam"
    else:
        p = ""  # TODO Is this really a valid case?

    tp = 0
    fp = 0
    for class_ in classes:
        if class_ == (k, k):
            tp += 1
        elif class_ == (k, p):
            fp += 1

    try:
        return tp / (tp + fp)
    except ZeroDivisionError:
        return 0