-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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: run doctests on travis #19952
DOC/CI: run doctests on travis #19952
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19952 +/- ##
=======================================
Coverage 92.05% 92.05%
=======================================
Files 169 169
Lines 50709 50709
=======================================
Hits 46679 46679
Misses 4030 4030
Continue to review full report at Codecov.
|
There were failures https://travis-ci.org/pandas-dev/pandas/jobs/347769808#L6516, but they build still passed. |
Hmm, that is because the script is still called itself from another script that does On the other hand, it is maybe better to move this to a separate script anyhow. |
0b810b4
to
10cf824
Compare
OK, this is working now, in the sense that travis is failing because of the failing doctests. The question is now how to list the ones we want to test:
Checking how it would look like with the second way |
Hmm, so running the doctests on travis gives segfaults for some of them (don't have this locally):
Somebody any idea? |
So this still has a segfault when running the doctests. I cannot reproduce it locally with pytest (
|
@jreback @TomAugspurger @datapythonista @WillAyd any comments? This enable doctests on travis, and for now runs tests for frame/generic/series.py. |
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.
small comments. generally +1 on this. Is there a doc about this in contributing? (certainly can do in later PRs), in particular formatting gotchas? Also list a couple of gold-star examples (in our current docs) that show good contstruction of these tests, so we can simply point to them and say, like this!
Once this is merged we can look to add these on new PRs and pls open a tracking issue for existing doc-tests to add to the tester
ci/doctests.sh
Outdated
# fi | ||
|
||
|
||
# # top-level reshaping functions |
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.
are these just not-working tests?
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.
Well, they are failing, and don't want to start fixing all of them here. Will remove or either skip the failing ones
I think it'll be great to have this, but it'll take a while until we have all doctests passing (it's in my TODO list, together with fixing all warnings from sphinx). I'm happy to merge this PR as it is, and keep adding docstrings and files as we fix the tests. |
Yes, we have a very elaborate docstring guide: http://pandas.pydata.org/pandas-docs/stable/contributing_docstring.html#docstring, which also contains some tips to get the doctests passing (it might need some better linking in the contributing docs though) |
for some tips and tricks to get the doctests passing. | ||
|
||
When doing a PR with a docstring update, it is good to post the | ||
output of the validation script in a comment on github. |
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.
@datapythonista eventually we should rework the documentation guidelines to include more of sprint content like https://python-sprints.github.io/pandas/guide/pandas_pr.html, but for now added short section more clearly mentioning the validation script and that the doctests need to pass
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.
Looks great, there is just a small typo.
doc/source/contributing.rst
Outdated
----------------------------- | ||
|
||
When improving a single function or method's docstring, it is not necessarily | ||
needed to build to full documentation (see next section). |
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.
s/to/the
e760f98
to
25bc158
Compare
Trying to see if we can check the doctests on travis. We were already running them, but at this point they were not failing travis if they failed.
Closes #19934
Closes #3439