Fix matplotlib converter registering warning #26770
Conversation
Closes pandas-dev#26760 Tests were being skipped, because the module name changed.
Codecov Report
@@ Coverage Diff @@
## master #26770 +/- ##
==========================================
+ Coverage 91.71% 91.81% +0.09%
==========================================
Files 178 178
Lines 50755 50762 +7
==========================================
+ Hits 46552 46606 +54
+ Misses 4203 4156 -47
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #26770 +/- ##
==========================================
+ Coverage 91.87% 91.95% +0.08%
==========================================
Files 180 180
Lines 50712 50727 +15
==========================================
+ Hits 46590 46645 +55
+ Misses 4122 4082 -40
Continue to review full report at Codecov.
|
|
is there any way to actually test this? can run a subprocess tests and check for warnings in the output? |
jorisvandenbossche
changed the title
@TomAugspurger actually proposed a way to test this in #24964, but you were against adding it (the test was called separately from |
re-looking that's a good idea actually. @TomAugspurger can you re-add something like that? |
|
Gonna try subprocess first, and then that if subprocess fails.
…On Tue, Jun 11, 2019 at 7:56 AM Jeff Reback ***@***.***> wrote:
is there any way to actually test this?
@TomAugspurger <https://github.com/TomAugspurger> actually proposed a way
to test this in #24964 <#24964>,
but you were against adding it (the test was called separately from
ci/run_tests.sh. Although that indeed adds complexity to the CI testing
code, this is only temporary and in a place (run_tests.sh) that does not
need to be touched often. So personally, I would just add it like in the
original version of that PR.
re-looking that's a good idea actually. @TomAugspurger
<https://github.com/TomAugspurger> can you re-add something like that?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#26770?email_source=notifications&email_token=AAKAOITMUOOXFGNFBI6PNTTPZ6OGRA5CNFSM4HWVLID2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXNASZA#issuecomment-500828516>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAKAOIV4HWUIALZKG5BXULDPZ6OGRANCNFSM4HWVLIDQ>
.
|
|
Subprocess seems to work pretty well here. |
|
The failures seem related (as they are about plotting), but not sure what change in this PR caused those |
|
For posterity, my current hypothesis is that
d03a41d may have a warning in the log. Not sure how to fix yet. Possibly, |
But this is something we can fix? I suppose this was introduced in the PR with the matplotlib split |
|
Yep. 28aaecf got them all I think. |
|
The warning was being emitted by matplotlib: https://travis-ci.org/pandas-dev/pandas/jobs/546786281#L2428 (that's before 28aaecf) |
|
OK, all green if you have another chance to look @jorisvandenbossche. Basically, we can / need to have And again, this is all going away in a release or so. In my debugging, I've compat code like https://github.com/pandas-dev/pandas/pull/26770/files#diff-53e8fd9a1f847dd40d9aba697909409fL524, which no longer seem to be tested, and https://github.com/pandas-dev/pandas/pull/26770/files#diff-53e8fd9a1f847dd40d9aba697909409fL1096, which isn't needed for any recent matplotlib version. |
|
thanks @TomAugspurger merged, so @datapythonista can rebase the backend PR |
Closes #26760
Tests were being skipped, because the module name changed in https://github.com/pandas-dev/pandas/pull/18307/files.