-
Notifications
You must be signed in to change notification settings - Fork 876
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix for issue 586 #588
fix for issue 586 #588
Conversation
Hello @sohrabtowfighi! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2019-09-15 22:42:57 UTC |
Thanks for the PR! Since we have no unit tests for the behavior of the plotting functions, could you update the examples in the jupyer notebook documentation to assess whether the changes work as intended? The respective file would be in https://github.com/rasbt/mlxtend/blob/master/docs/sources/user_guide/plotting/plot_decision_regions.ipynb |
I ran the file you said. Looks the same except I got a FileNotFoundError at the very last line. I can't find the file and this seems unrelated to my commit. In [23]: |
Oh that's because those are not explicitly saved on/commited to the GitHub repo (to save space). These are the function and class documentations that get directly extracted from the docstrings. So, all you would need to do is a) cd into the docs subdirectory, for example
b) in this folder, run the
This will generate all the up-to-date |
I got this error File "C:\Users\sohra\Anaconda3\lib\site-packages\skimage\util\arraycrop.py", line 8, in I also don't think this is related to my commit and I have a fixed copy of plot_confusion_matrix already in my code. I don't think I will go further in this rabbit hole. |
Hm weird. In any case, no need to dig into that further. Could you pls upload the updated jupyter notebook you mentioned? No need to execute the last cell (the one which caused the error); I can take care of that later |
Description
Related issues or pull requests
Pull Request Checklist
./docs/sources/CHANGELOG.md
file (if applicable)./mlxtend/*/tests
directories (if applicable)mlxtend/docs/sources/
(if applicable)PYTHONPATH='.' pytest ./mlxtend -sv
and make sure that all unit tests pass (for small modifications, it might be sufficient to only run the specific test file, e.g.,PYTHONPATH='.' pytest ./mlxtend/classifier/tests/test_stacking_cv_classifier.py -sv
)No test found. Basic run on my own end worked.
flake8 ./mlxtend
I don't have flake8. changes were minimal.