Skip to content

ENH: refactor of docteset plugin management #137

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

Closed
wants to merge 7 commits into from

Conversation

matthew-brett
Copy link
Contributor

We previously had a baroque inheritance scheme to deal with the case
where the user had normal nose doctests enabled in their environment.
However, this scheme didn't deal with bench() routine, and was
complicated. This commit uses a null Unplugger plugin to pull the
doctest plugin off the nose configuration after it has been initialized.
We can use this for bench() and test(), and it allows the doctest module
to be enabled (by the user environment) and then thrown away.

Also rejigged the docstrings and removed the automated docstring
addition as the docstrings have already been copied and adapted in the
code.

We previously had a baroque inheritance scheme to deal with the case
where the user had normal nose doctests enabled in their environment.
However, this scheme didn't deal with bench() routine, and was
complicated. This commit uses a null Unplugger plugin to pull the
doctest plugin off the nose configuration after it has been initialized.
We can use this for bench() and test(), and it allows the doctest module
to be enabled (by the user environment) and then thrown away.

Also rejigged the docstrings and removed the automated docstring
addition as the docstrings have already been copied and adapted in the
code.
Identifies the tests to run. This can be a string to pass to
the nosetests executable with the '-A' option, or one of several
special values. Special values are:
* 'fast' - the default - which corresponds to the ``nosetests -A``
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced the '*' is an improvement for reading in a terminal. But it isn't bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's probably neutral. The edit was actually because this markup in sphinx breaks the latex build, because the indentation here (I think I might have been the original author) makes sphinx attempt to make this into a quote, and this causes a 'too deeply nested' quote error in latex.

@charris
Copy link
Member

charris commented Aug 13, 2011

Looks good to me, but I'm not familiar with running doctests. Unless someone more knowledgeable comments I'll go ahead and commit this in a few days.

Refactor ``prepare_test_args`` method to make it easier for subclasses
to adapt its behavior.  This should make it easier for nipy and other
projects to use the numpy testing machinery without wholesale copies
into their source trees.
The doctesting tests were in the code file, and (for me) rather
difficult to run without running lots of other tests.  With this change
you can run the doctest tests in isolation by executing the
test_doctesting.py file.
The numpy doctest extension generates an error with empty doctest output;
this most often comes about with the +SKIP option.

The numpy doctest plugin exposed a nose bug because it accidentally used
a different default for the 'doctest-result-variable'.

nose bug report here:
http://code.google.com/p/python-nose/issues/detail?id=445
There are various docstrings show examples of how to run the tests, and
give example test output.  Obviously the test output changes, and
running the doctests for the testing package:

import numpy.testing as npt
npt.test(doctests=True)

will cause several large sets of tests to be run in the rest of the
tree.  So I skipped these.
@matthew-brett
Copy link
Contributor Author

Sorry to add more commits here. I think the additions a) follow on simply from the original pull request and b) are uncontroversial - but I'd be very glad for a review.

@charris
Copy link
Member

charris commented Aug 16, 2011

Should I let this simmer a bit more?

NumpyDocTestCase definition overwritten further down the file.

The deleted class only redefined the ``id`` method with the same code
as that in the parent class since before nose 0.10.
Move numpy-specific parts of the plugin into their own methods, or into
class-level defines.  This makes it easier to subclass the plugin.  This
in turn may help keep more eyes on the code.
@matthew-brett
Copy link
Contributor Author

On Mon, Aug 15, 2011 at 8:08 PM, charris
reply@reply.github.com
wrote:

Should I let this simmer a bit more?

As you twisted my arm - I've added a couple more commits :)

The first is just removing some unused code.

The second is a tiny bit more controversial, in that it has no
functional impact, but makes it easier to subclass the doctest plugin.
I think that's a good idea because it makes the plugin a nice way
into making your own doctest plugin class, which is not an easy job
from a standing start.

I think that's it though.

Thanks for the review and keeping me reminded.

@charris
Copy link
Member

charris commented Aug 16, 2011

OK then, pushed 5cf0a07..78f7542.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants