-
-
Notifications
You must be signed in to change notification settings - Fork 25.5k
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
CLN: remove nose fixtures in docs #10187
CLN: remove nose fixtures in docs #10187
Conversation
Good catch, thanks a lot!
I like a single |
@@ -65,7 +65,7 @@ def pytest_runtest_setup(item): | |||
setup_rcv1() | |||
elif fname.endswith('datasets/twenty_newsgroups.rst'): | |||
setup_twenty_newsgroups() | |||
elif fname.endswith('datasets/working_with_text_data.rst'): | |||
elif fname.endswith('tutorial/text_analytics/working_with_text_data.rst'): |
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.
Hmmm ... I am wondering why this actually worked before ...
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.
Oh well, I guess the 20 newsgroup data was just downloaded on Travis.
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.
Yep see https://travis-ci.org/scikit-learn/scikit-learn/jobs/305782018#L2491 where doc/tutorial/text_analytics/working_with_text_data.rst
was run (.
instead of s
)
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.
And now it is skipped again:
https://travis-ci.org/scikit-learn/scikit-learn/jobs/305841207#L2491
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.
I was also wondering that! :-)
LGTM, merging, thanks a lot! No need to wait for AppVeyor since don't run the doctests from the doc on AppVeyor. |
Reference Issues/PRs
Follow-up on #9697 and #9840
This removes the
_fixtures.py
in the docs which were used by nose. The pytest equivalent is theconftest.py
file that @lesteve already added before.@lesteve I moved the conftest.py one level up to the root of the docs, so we can use one for all docs. I can also leave it in
doc/datasets/
, and then add another conftest.py indoc/tutorial/
,doc/modules/
if you prefer that.