-
-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
CI: Remove warning raising after new matplotlib release #31573
Conversation
xref #30588 |
import matplotlib as mpl | ||
|
||
version = mpl.__version__ | ||
if version > "3.1.2": |
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.
use LooseVersion
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.
can also just define a new mpl skip decorator and skip if < 3.1.2
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.
thanks for your quick reply, changed, and let me know if this is what you meant @jreback
@@ -59,16 +60,15 @@ def test_register_by_default(self): | |||
call = [sys.executable, "-c", code] | |||
assert subprocess.check_call(call) == 0 | |||
|
|||
@td.skip_if_low_mpl |
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.
yes right idea
i think we have some skips that are based on mpl versions already (maybe in the other test plotting files)
can u name like them
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.
emm, i see we have skip_if_no_mpl
and skip_if_mpl
decorators, but seems no version involved.
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.
does skip_if_old_mpl
sound better? or skip_if_lower_3_1_2_mpl
?
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.
latter
Hello @charlesdong1991! 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 2020-02-02 16:12:22 UTC |
@@ -91,6 +91,7 @@ def test_matplotlib_formatters(self): | |||
assert Timestamp not in units.registry | |||
assert Timestamp in units.registry | |||
|
|||
@td.skip_if_lower_3_1_2_mpl |
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.
sorry we don't really want to add this as a full decorator, just do this
@td.skip_if_no("matplotlib", min_version="3.1.2")
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.
comments, ping on green.
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.
great. ping on green.
this looks better simpler, i didn't notice this since it uses |
ping @jreback |
thanks for pro-actively fixing this @charlesdong1991 ! |
…tplotlib release
…elease (#31576) Co-authored-by: Kaiqi Dong <kaiqidong1991@gmail.com>
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff
seems CI is broken after the new release of matplotlib