Fix permutation importance failing when target is categorical

[angela97lin]

Closes #3012

[codecov]

Codecov Report

Merging #3017 (2c1f99c) into main (79b9d22) will increase coverage by 0.1%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##            main   #3017     +/-   ##
=======================================
+ Coverage   99.7%   99.7%   +0.1%     
=======================================
  Files        312     312             
  Lines      30137   30143      +6     
=======================================
+ Hits       30041   30047      +6     
  Misses        96      96             

Impacted Files	Coverage Δ
...alml/model_understanding/permutation_importance.py	100.0% <100.0%> (ø)
...understanding_tests/test_permutation_importance.py	100.0% <100.0%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79b9d22...2c1f99c. Read the comment docs.

[jeremyliweishih]

nice!

[eccabay]

LGTM, thanks for picking this up! Super useful in enabling human readable pipeline explanations :)

[chukarsten]

Do we need to address the classification aspect that freddy mentioned in the issue?

[freddyaboulton]

Looks good to me @angela97lin !

[freddyaboulton]

I think we're ok for classification here. In _fast_permutation_importance we encode the target with the pipeline._encode_targets method. So the estimator predictions will also be encoded.

Of course, if the pipeline does not have an encoder but has string-valued targets this would fail but I would say it's a pipeline definition bug as opposed to a permutation importance bug.

The fact that our classification objectives only supports integer-valued targets and that to score a pipeline it should have an encoder may not be super clear though.

[angela97lin]

Agreed! I wish there were a way to make this more clear. Pipelines require a label encoder for not only scoring but also fit so it still feel consistent, but I wonder if moving forward we could make these types of error messages more clear.