Confusion when using class ChordEvaluation

[expectopatronum]

Expected behaviour

When looking at the docs for ChordEvaluation I expected that I need to pass the file paths to the annotation and prediction files.

The doc says:

    detections : str
        File containing chords detections.
    annotations : str
        File containing chord annotations.

Actual behaviour

It actually expects

chord_labels : numpy structured array
        Chord segments in `madmom.io.SEGMENT_DTYPE` format

like encode() for example.

Steps needed to reproduce the behaviour

from madmom.evaluation.chords import ChordEvaluation

pred_path = "queen_greatest_hits_i_01_bohemian_rhapsody.preds"
annotation_path = "queen_greatest_hits_i_01_bohemian_rhapsody.chords"

eval = ChordEvaluation(pred_path,
                       annotation_path) 

results in

  File "/home/verena/miniconda3/envs/py37-iml/lib/python3.7/site-packages/madmom/evaluation/chords.py", line 753, in __init__
    self.ann_chords = merge_chords(encode(annotations))
  File "/home/verena/miniconda3/envs/py37-iml/lib/python3.7/site-packages/madmom/evaluation/chords.py", line 66, in encode
    encoded_chords['start'] = chord_labels['start']
TypeError: string indices must be integers

the following works:

from madmom.evaluation.chords import ChordEvaluation, load_chords

pred_path = "queen_greatest_hits_i_01_bohemian_rhapsody.preds"
annotation_path = "queen_greatest_hits_i_01_bohemian_rhapsody.chords"

eval = ChordEvaluation(load_chords(pred_path),
                       load_chords(annotation_path))

I suggest either an update to the documentation or calling load_chords inside ChordEvaluation in case you expect the user to pass the files, but from how this test is implemented I assume the implementation is correct and documentations needs an update.

If you agree I can take care of fixing it! (I didn't take a very close look yet but it seems not all Evaluation classes are implemented the same way, but at least KeyEvaluation seems to have the same issue)

[superbock]

Sorry for the late response, yes the documentation should be updated. However, I would also merge a patch which adds loading of chords in a try/except block for convenience — but still the docs should be updated to reflect this as well.

I see that we did not take too much care about the docstrings all over the whole evaluation module. We should fix the other classes as well.

[fdlm]

Also late to the party! Thanks for finding this. Yes, the docs are wrong and should be updated. In a next step, we could change all eval classes so that they accept filenames as well.