weecology / DeepForest

Python Package for Airborne RGB machine learning
https://deepforest.readthedocs.io/
MIT License
521 stars 176 forks source link

Allow multi-class plot in plot_results #791

Closed bw4sz closed 1 month ago

bw4sz commented 1 month ago

There was an error in the multi-class logic.

https://github.com/weecology/DeepForest/issues/790

This introduces a simple fix to check the object type and a test to assert this function.

ethanwhite commented 1 month ago

I also just got an update merged into supervision so this will work as originally written with the next release: https://github.com/roboflow/supervision/pull/1550 (but we should go ahead and make the change I mentioned above and then we can change it back later)

bw4sz commented 1 month ago

The suggested change raises

import os
import pandas as pd
from deepforest import get_data
from deepforest import visualize
from deepforest.utilities import read_file

results = pd.read_csv(get_data("testfile_multi.csv"))
results = read_file(results, root_dir=os.path.dirname(get_data("SOAP_061.png")))
visualize.plot_results(results)
>>> visualize.plot_results(results)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/benweinstein/Documents/DeepForest/deepforest/visualize.py", line 405, in plot_results
    if num_labels > 1 and len(results_color_sv.colors) != num_labels:
                              ^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Color' object has no attribute 'colors'
supervision               0.22.0                   pypi_0    pypi

Maybe it works with different versions, but we should steer clear of anything that sensitive to version in a dependency.

bw4sz commented 1 month ago

I'm confused by the change here. It checks for a multi-class context and then only (and always) sets the color palette if the user has passed in a color palette. It seems like that means that: 1) the multi-color palette will only be set if the user passes a color palette (i.e., not if they leave the default); and 2) it will always override a user specified set of multi-class colors. Am I misunderstanding something?

I think you are right here, it should be

    if num_labels > 1 and not type(results_color_sv) == sv.ColorPalette:

if the user has multi-class labels, and has not already passed a colorPalette, make a color palette.

ethanwhite commented 1 month ago

Oops, sorry. The multiple types thing is definitely a little confusing to keep track of. I do think we still want to consider the length of the provided color palette if provided. I just pushed an (actually tested) edit that refactors a little to put all of the type specific code together at the beginning and hopefully get exactly the behavior that we want. See what you think.

bw4sz commented 1 month ago

looks good to me, merge when ready.