Add styles parameter to manual_legend #1094
Conversation
Added styles parameter to manual_legend per issue #566. With tests and bug fix for existing manual_legend test.
Sets up tests/test_gridsearch with basic tests based on sample .ipynb. Only one change to gridsearch code (quick method return changed from None to visualizer).
There was a problem hiding this comment.
Hello again @tktran — first off, my sincere apologies for the slow response, it's been a busy few months!
Secondly, thank you so much for your contribution! I took a preliminary look at your PR and the test failures on Travis, and it looks like they're caused by a few unused imports.
Are you still interested in working on this PR? I noticed that the branch that you were working in appears to have been deleted, but if you can recover it and make these small updates, we can proceed with reviewing your PR!
| X, y = load_occupancy(return_dataset=True).to_pandas() | ||
| X, y = X.head(1000), y.head(1000) | ||
|
|
||
| gs = self.gridsearchcv |
There was a problem hiding this comment.
Looks like this is one of the places that is causing the Travis errors; would you mind removing this unused import?
| X, y = load_occupancy(return_dataset=True).to_numpy() | ||
| X, y = X[:1000], y[:1000] | ||
|
|
||
| gs = self.gridsearchcv |
There was a problem hiding this comment.
Looks like this is another one of the places that is causing the Travis errors; would you mind removing this unused import as well?
| import pytest | ||
|
|
||
| from tests.base import VisualTestCase | ||
| from tests.fixtures import Dataset, Split |
There was a problem hiding this comment.
Looks like the Split import is not used in the file; would you mind removing it to resolve the unused imports errors on Travis?
|
Please see this PR redone at #1117. This PR that you are looking at is a little messy, since you are reviewing both code that I committed as part of issue #556 (as intended) and issue #308 (not intended, should have been isolated to its corresponding PR #1097). The new PR leaves the latter code out. Thanks for your patience. |
|
@rebeccabilbro And thank you for taking the time to review my code. You may be seeing a few more contributions from me in the coming months, as I attempt to build up a history of contributions to Numfocus projects in preparation for applications to GSoC 2021. (But I do already know that yellowbrick won't be participating). |
This PR is intended to fully implement issue #566. It includes unit tests for the new feature, and a bug fix for the existing manual_legend test (which failed to enable drawing the legend in the test image). It does not include updated documentation, which I would gladly begin working on upon approval of this PR. Also open to your suggested changes.
Questions for the @DistrictDataLabs/team-oz-maintainers:
Checklist:
pytest?