-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
DOC/CI: Failing documentation build on warnings #26852
DOC/CI: Failing documentation build on warnings #26852
Conversation
Codecov Report
@@ Coverage Diff @@
## master #26852 +/- ##
==========================================
- Coverage 91.88% 91.87% -0.01%
==========================================
Files 179 179
Lines 50696 50696
==========================================
- Hits 46581 46576 -5
- Misses 4115 4120 +5
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #26852 +/- ##
==========================================
- Coverage 91.87% 91.86% -0.01%
==========================================
Files 180 180
Lines 50710 50712 +2
==========================================
- Hits 46588 46585 -3
- Misses 4122 4127 +5
Continue to review full report at Codecov.
|
It's failing! So it is working :) |
Yes, first test passed. Also, it's failing with return code 2, both sphinx and the ipython hack are failing (I test that well locally anyway, but good to see the CI red) :) pushing some fixes soon, will get the remaining warnings fixed here, so when this is green, we're done :) |
@property | ||
def asi8(self): | ||
""" | ||
Integer representation of the values. |
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.
Only for the datetime-like indexes (we should probably rather remove it, but that's another issue)
Not sure about the clipboard problem, it's not immediate to fix. I'd merge this PR with the fix proposed in #26700. Or if anyone else wants to give a try to set up |
I am fine with that. We know now that clipboard doesn't work on Azure and so no clipboard tests are running there, so we can open an issue about that instead of keeping a failing example in the docs. |
i think we should get them running first |
I created #26852/ for the issue. Will close this then, but I don't think we're being smart here. By the time the issue is fixed we'll probably have introduced several new warnings as it happened last time we spent several days addressing them. Quite demotivating besides the extra work... |
broken tests are much worse |
The tests not being run are not related to the documentation build. It's thanks to the docs that we now know they are not run. But now we know it, an open issue is just as good a reminder to fix it as a failing example in the docs. Marc and I put a lot of effort in getting to this point of a warning-free doc build. The clipboard example is only one left. I think it is important to finally actually check the doc build in CI to prevent we constantly introduce new warnings. |
I think we all agree that we need to fix the tests. But the tests will remain skipped whether we validate the warnings in the docs or not. I don't see the gain in blocking here. But anyway, it's just some doc warnings, not a big deal anyway. |
Marc, don't dismiss your own work, a correct sphinx build is relevant. |
@datapythonista i am actually ok merging this having a healthy CI is he most important thing |
Thanks Jeff. Agree with prioritizing the CI, will do more tests in a separate PR, but I already spent few hours with the clipboard, and it's not trivial. It'll probably take a while, no idea what's the problem. That's why I think doesn't make sense to block this which is unrelated and I'd say urgent (really don't want to start getting new warnings added again after all the work). This is passing the CI now, validating that no sphinx or ipython directive warnings are raised during the doc build. |
@pandas-dev/pandas-core be aware that once this is merged, warnings in the documentation will leave the CI red. |
Thanks for working on this Marc! It’s been quite an effort.
… On Jun 16, 2019, at 17:00, Marc Garcia ***@***.***> wrote:
@pandas-dev/pandas-core be aware that once this is merged, warnings in the documentation will leave the CI red.
—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
||
df = pd.DataFrame(np.random.randn(5, 3)) | ||
df | ||
df.to_clipboard() | ||
pd.read_clipboard() | ||
>>> df = pd.DataFrame({'A': [1, 2, 3], |
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.
The lines above needed to be removed I think?
Let's give this a go! |
It might also be advisable to be extra careful to merge master in PRs before merging the coming days, certainly for PRs that touch the documentation. |
Thanks for the fixup Joris. This failed in master, looks like the scipy documentation is down (we fetch https://docs.scipy.org/doc/numpy/objects.inv for the cross references). @rgommers not sure if you're aware? |
Yes, noticing this locally as well. |
Thanks for the ping. No I didn't notice that yet. Will follow up on that now. |
scipy docs are now working, and the doc build is green again. |
git diff upstream/master -u -- "*.py" | flake8 --diff