Skip to content
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

Splits prediction error plots from residuals #1078

Merged
merged 4 commits into from
Jul 7, 2020

Conversation

rebeccabilbro
Copy link
Member

@rebeccabilbro rebeccabilbro commented Jun 14, 2020

This PR closes #1031, splitting prediction error plots from residuals.

I have made the following changes:

  1. Created new files, yellowbrick/regressor/prediction_error.py and tests/test_regressor/test_prediction_error.py and migrated code there from yellowbrick/regressor/residuals.py and tests/test_regressor/test_residuals.py, respectively.
  2. Relocated the relevant baseline images into a new directory, tests/baseline_images/test_regressor/test_prediction_error
  3. Updated yellowbrick/regressor/__init__.py
  4. Updated the docs accordingly

CHECKLIST

  • Is the commit message formatted correctly?
  • Have you noted the new functionality/bugfix in the release notes of the next release?
  • Included a sample plot to visually illustrate your changes?
  • Do all of your functions and methods have docstrings?
  • Have you added/updated unit tests where appropriate?
  • Have you updated the baseline images if necessary?
  • Have you run the unit tests using pytest?
  • Is your code style correct (are you using PEP8, pyflakes)?
  • Have you documented your new feature/functionality in the docs?
  • Have you built the docs using make html?

Copy link
Contributor

@Kautumn06 Kautumn06 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great @rebeccabilbro! I was able to build the docs and checked to make sure the images generated correctly. I've updated the branch and once the tests have passed, I can merge it in. Thanks again!

@rebeccabilbro rebeccabilbro merged commit bc693dc into DistrictDataLabs:develop Jul 7, 2020
@rebeccabilbro
Copy link
Member Author

Thanks for the review @Kautumn06! Something weird must have been up with Travis's Miniconda image because the build has been failing the last couple weeks (even though the Appveyor Miniconda was ok). But it looks like they must have sorted it out on their end, so now the tests pass as expected! 🎉

@rebeccabilbro rebeccabilbro deleted the peplot-split branch July 7, 2020 17:52
@Kautumn06
Copy link
Contributor

Thank you @rebeccabilbro! I tried to figure out the issue with the Minicoda image last weekend, but admittedly testing is not my strong suit. I was at a loss about what the problem could be, so on Tuesday I actually just went on Travis CI and tried the only thing I could think of — which was to rerun the tests and hope for the best 🙏

I'm glad that the tests are now passing as expected and thank you for merging it in!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API reference conflict for residuals.py in documentation
2 participants