-
Notifications
You must be signed in to change notification settings - Fork 16
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
Ignore "You are using [hist/kde] as a plot kind" warnings #342
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #342 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 34 34
Lines 2858 2858
=========================================
Hits 2858 2858
☔ View full report in Codecov by Sentry. |
These are warnings that I'd argue should pop up in the tests, so they should not be suppressed, and they should "affect our quality of life"... The warnings are bound to go away eventually with #320, but until then they are a good reminder that we should eventually clean this up fully. |
I agree they will go away with #320 as it stands, but should they? Unless we actually make hist and kde inaccessible through anesthetic they will still "look odd" |
Agreed, it is probably good to keep them, but that means that rather than just filtering them out of the tests, we should make them part of the tests, i.e. make use of I see two ways of implementing this:
What would others think best? |
Seeing as the filters don't appear that many times in the test suite, I think 1 Edit: ends up being a bit awkward since |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, thanks @AdamOrmondroyd. Please, squash and merge if you think it is ready. And please feel free to hit "request review" to get a faster reaction from me.
Description
Since the
UserWarning
s were added to prevent users from unintentionally usingkde
instead ofkde_1d
etc, the test suite has been (correctly) flagging up these warnings, affecting quality of lifeTests obviously won't pass until either #341 or #339 are merged.
Fixes # (issue)
Checklist:
flake8 anesthetic tests
)pydocstyle --convention=numpy anesthetic
)python -m pytest
)