Fixes #812

[ocraft]

Fix for #812

[dberenbaum]

Suggested change

	k: v for k, v in kwargs.items() if k not in plot_config or k == "normalized"
	k: v for k, v in kwargs.items() if k not in plot_config

We don't need any check for normalized here. It's already part of plot_config, which is all that's needed. We don't want to include it in sklearn_kwargs since it's purely to set the dvc plots template to confusion_normalized.

[dberenbaum]

Thanks @ocraft! Ideally, I would like to make title, x_label, y_label, and normalized all named arguments to log_sklearn_plot() for a couple reasons:

1. It looks like all of these are undocumented and hidden now (I'm curious how you found out how to use them)
2. The docs state that kwargs are passed to the underlying sklearn methods, which is inaccurate since it is really a mix of these dvc plot parameters and sklearn args. It would be better to keep these separate rather than rely on this implicit parsing of the kwargs.

If you are interested, feel free to try this, but I'm fine to merge without that if it's beyond what you want to take on.

[ocraft]

I think that I can do that. Personally, I just expected that log_plot and log_sklearn_plot would have the same arguments; it seemed logical to me.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.52%. Comparing base (602c053) to head (c7b66ef).
Report is 10 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #813      +/-   ##
==========================================
+ Coverage   95.47%   95.52%   +0.04%     
==========================================
  Files          57       57              
  Lines        3889     3951      +62     
  Branches      353      353              
==========================================
+ Hits         3713     3774      +61     
- Misses        124      125       +1     
  Partials       52       52              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dberenbaum]

Suggested change

	sklearn_kwargs = {k: v for k, v in kwargs.items() if k not in plot_config}
	plot.step = self.step
	plot.dump(val, **sklearn_kwargs)
	plot.step = self.step
	plot.dump(val, **kwargs)

Since the plot config options are now named args, I don't think it should be necessary to do anything here but pass kwargs directly.

[dberenbaum]

@ocraft Please see the one comment. If you can ping me once it's ready, it should be good to approve after that. Thanks again!

[ocraft]

@dberenbaum Done :)