Add an example directive to mark in-text examples for the example gallery #22

jonathansick · 2019-06-30T20:55:13Z

This PR is the first part of the example gallery work described in astropy/astropy#7242. It adds an example directive that marks examples in documentation text. For example:

.. example:: Title of the example
   :tags: first-tag, another-tag

   Content of the example.

   More content in the example.

This content is not part of the example.

The example directive does not change the visual appearance of the example content in the documentation text, but it does record the example in the build environment so that it can be republished in the example gallery (work to be done).

Example metadata

Internally, the example directive stores examples in the build environment under the sphinx_astropy_examples attribute. Examples are keyed by example-src-*slugified-title* and metadata for each example consists of:

title
tags (set of tag strings)
contents (unparsed content of the directive)
docname
lineno

Target node

The example directive also adds a target node to the start of the example content in the original content source. This will make it possible to backlink to the original source of the example.

Testing

For this work I'm using the same testing strategy that Sphinx is using for its extensions. The idea is that we add mock Sphinx projects to sphinx_astropy/tests/roots. Each subdirectory is an isolated example site. These sites are associated with a test module. Test functions use a pytest.mark.sphinx directive to do isolated Sphinx builds where we can test the output in various formats (html, latex, xml, or "dummy" to just see docutils nodes):

@pytest.mark.sphinx('dummy', testroot='example-gallery')
def test_example_env_persistence(app, status, warning):
    """Test that the examples are added to the app env.
    """
    app.builder.build(['docname'])
   # test code...

The downside of this test strategy is that it doesn't work (as far as I can tell) with Sphinx 1.6, so I'm having pytest ignore these tests in the Sphinx 1.6 build. I suggest this trade-off is ok because it makes it convenient to realistically test Sphinx extensions in all newer versions of Sphinx.

This package will gather all Sphinx extensions related to the example gallery. There will be two main parts: a directive that marks examples, and extensions that index and render those examples. This boilerplate includes the standard Sphinx setup function for the extension.

This includes the sphinx_astropy.ext.example Sphinx extension by default in Sphinx builds.

This directive marks the scope of example content in the original documentation, and lets authors add a title and tags. Currently a pass-through directive. It parses the content of the directive and adds it back to the document Examples are persisted in the build environment for later post-processing (to build the example gallery). Examples are keyed by their unique ID (a slugified version of the title). The dict items contains metadata and the content of the example (to later generate standalone example pages). The directive also collects title and tags as metadata.

This target node lets us backlink to the example in the main documentation. In html, the link is an id on the first element of the example content. example IDs are unique since they're the keys of the env.sphinx_astropy_examples dictionary in the environment.

sphinx.testing.fixtures let us build Sphinx sites from pytest and then inspect the built site. There can be multiple test sites, each test site is a directory in the sphinx_astropy/test/roots/ directory. This is the same pattern that Sphinx uses for its test, so this is likely the easiest way to test our Sphinx extensions. http://www.sphinx-doc.org/en/master/devguide.html#unit-testing Note I had to add the pytest_plugins line, to load the Sphinx pytest plugin, from a new conftest.py file at the root of the project, not from sphinx_astropy/tests/conftest.py The reason for this is outlined in https://docs.pytest.org/en/latest/deprecations.html#pytest-plugins-in-non-top-level-conftest-files The rest of the pytest configuration is done in sphinx_astropy/tests/conftest.py, which is where you'd expect most configuration to go. This configuration is largely based on Sphinx's: https://github.com/sphinx-doc/sphinx/blob/master/tests/conftest.py

The example-marker.rst file contains several instances of the example directive, testing different conditions (having tags, or not, and having different types of content in the example).

This test generates a site in the XML format since then it's easy to search for nodes and their attributes. Unfortunately this test strategy doesn't work for Sphinx <1.7, because the pytest fixtures aren't available. Thus I have pytest skip these tests for Sphinx <1.7. I think this is still the best way to test sphinx extensions and will continue to be so in the future because this is how Sphinx tests itself.

bsipocz · 2019-07-01T23:19:31Z

The downside of this test strategy is that it doesn't work (as far as I can tell) with Sphinx 1.6, so I'm having pytest ignore these tests in the Sphinx 1.6 build. I suggest this trade-off is ok because it makes it convenient to realistically test Sphinx extensions in all newer versions of Sphinx.

What does "doesn't work" mean? It does nothing, or there are errors?
What is the oldest sphinx it would still work? Depending on how long ago it was released, it may make sense to bump the minimum required version.

bsipocz · 2019-07-01T23:20:53Z

Also, would this work in a docstring, too? If yes please add a test for it.

jonathansick · 2019-07-02T01:35:05Z

What does "doesn't work" mean? It does nothing, or there are errors?

Sorry I was superficial about that. This is what I see (circle ci link):

sphinx_astropy/tests/test_example.py E                                   [100%]

==================================== ERRORS ====================================
_______________ ERROR at setup of test_example_directive_targets _______________

request = <SubRequest 'test_params' for <Function test_example_directive_targets>>

    @pytest.fixture
    def test_params(request):
        """
        test parameters that is specified by 'pytest.mark.test_params'
    
        :param Union[str] shared_result:
           If the value is provided, app._status and app._warning objects will be
           shared in the parametrized test functions and/or test functions that
           have same 'shared_result' value.
           **NOTE**: You can not specify shared_result and srcdir in same time.
        """
>       env = request.node.get_marker('test_params')
E       AttributeError: 'Function' object has no attribute 'get_marker'

/opt/conda/envs/sphinx16/lib/python3.6/site-packages/sphinx/testing/fixtures.py:89: AttributeError
=============================== warnings summary ===============================

Sphinx 1.7 is the oldest version in the test matrix that's working. Now, it's entirely possible it's just a small API mismatch and we could conditionally patch it. I haven't spent any real time looking into it yet.

Do you have a big user base on Sphinx 1.6 still? Or have most projects moved on to Sphinx 1.7/1.8/2.x.

Also, would this work in a docstring, too? If yes please add a test for it.

Good point. I hadn't thought of that! I'll check and test that.

bsipocz · 2019-07-02T02:17:19Z

Sphinx 1.7 was released in Feb 2018, so it sounds OK to me to bump the version, at least for the directive.

@astrofrog - what do you think?

adrn · 2019-07-02T17:38:10Z

This looks great to me - I don't have any comments!

astrofrog

This looks good to me, though I have a few comments below. I've also re-activated CircleCI so hopefully the tests will run now. Dropping Sphinx 1.6 seems reasonable.

astrofrog · 2019-07-04T17:10:01Z

sphinx_astropy/conf/v1.py

@@ -133,7 +133,8 @@ def check_sphinx_version(expected_version):
    'sphinx_astropy.ext.doctest',
    'sphinx_astropy.ext.changelog_links',
    'sphinx_astropy.ext.missing_static',
-    'sphinx.ext.mathjax']
+    'sphinx.ext.mathjax',
+    'sphinx_astropy.ext.example']


Swap order so that the astropy ones are together

astrofrog · 2019-07-04T17:13:50Z

sphinx_astropy/tests/conftest.py

+def rootdir():
+    """Directory containing Sphinx projects for testing.
+    """
+    return path(__file__).parent.abspath() / 'roots'


why 'roots'? in sphinx-automodapi we use 'cases', so that might be clearer?

I agree that cases is the better word here. roots comes from how Sphinx does their testing (see Sphinx's tests/roots directory).

I can rename the roots directory to cases, but there'll still be the word root in the name of this fixture and in the testroot parameter to @pytest.mark.sphinx.

The rootdir() fixture is picked up by the pytest.mark.sphinx's testroot parameter to build the absolute path to a given Sphinx test case:

@pytest.mark.sphinx('xml', testroot='example-gallery') def test_example_directive_targets(app, status, warning): # ...

@astrofrog, so are you fine with the directory being cases but using keeping the roots terminology from the Sphinx?

Yes that sounds good! 👍

astrofrog · 2019-07-04T17:14:19Z

sphinx_astropy/ext/example/marker.py

+        if self.example_id in env.sphinx_astropy_examples:
+            raise SphinxError(
+                'There is already an example titled "{self.title}" '
+                '({self.docname:self.lineno})'.format(self=self.title))


Can you add a test that exercises this?

Whoops I really messed up that untested error handling! I've fixed this code and added a test case.

There is a new test case (test-example-gallery-duplicates) because otherwise the SphinxError would always be raised for regular testing of the example directive.

As the docstring comment says, I found a weird case that while enabling the numpydoc extension in the test environment, I would get false alarms about duplicate examples already in the build environment. These duplicate examples came from other test functions. Somehow the environment is being preserved across builds now that numpydoc is activated. To make the ExampleMarkerDirective robust against this case, it's now making sure that the duplicate instance is from a different document and line number before raising a SphinxError.

If the tests are failing, it's useful to see the debug-level logging. Note: the '2' verbosity enables DEBUG-level logging. I can't find a cleaner alias to this. Also, it would be nicer to make this the default while using the sphinx pytest mark, but I can't find an easy way to do that.

Now the test-example-gallery root is using an autodoc+numpydoc processing pipeline in its build configuration. This confirms that the directive does work in a docstring as expected.

jonathansick · 2019-07-06T22:49:08Z

Also, would this work in a docstring, too? If yes please add a test for it.

@bsipocz I've added a test case with an autodoc+numpydoc pipeline and I've confirmed that the directive is processed as usual. That said, while talking to @kelle we might want to discourage example directives in docstrings as a matter of policy as there's some concern that examples in docstrings may not be as crafted as example snippets coming from the main documentation.

While testing for compatibility with numpydoc I did encounter a weird situation with the build environment being persisted across test functions, which triggers the code that prevents duplicate examples. (Just enabling the numpydoc extension in conf.py triggers this condition). I've made the duplicate detection code in ExampleMarkerDirective more nuanced so that it accepts having a duplicate example from exactly the same document and line number, as that likely means the example is coming from an earlier build in the tests.

bsipocz · 2019-07-06T23:03:35Z

We already have quite a lot of numpydoc Examples, with many of them sufficiently standalone and complete to be transformed to an example with the new directive (e.g. stats, periodograms, table, have many of those).

kelle · 2019-07-08T16:24:45Z

Started a new issue (astropy/astropy-tutorials#378) for discussing whether or not we want to put examples from docstrings into the Example Gallery. I'm totally fine with the functionality existing, I'm just not convinced we should use it.

bsipocz · 2019-07-08T16:54:31Z

I'm just not convinced we should use it.

I'm convinced that we shouldn't use it for all the cases. But some are nice enough (but then maybe they should be moved out of the docstrings, I don't know).

jonathansick · 2019-07-08T23:58:40Z

I just realized I need to add some event handlers because the directive is storing state in the environment:

env-merge-info (to be parallel read-safe)
env-purge-doc

I'll take care of these shortly.

eblur · 2019-07-09T17:58:33Z

Can we add authors as optional metadata?
(Sorry to throw this in last minute. It could be an option after the main PR is done)

jonathansick · 2019-07-09T19:11:02Z

@eblur we could add an authors field, or any other relevant metadata fields.

Is authorship something that astropy wants to start displaying? I see that the tutorials in learn.astropy.org have authors, but the "main" docs.astropy.org docs don't.

bsipocz · 2019-07-10T18:55:30Z

How do you define authors? The first one who adds an example or everyone who touches those examples?

eblur · 2019-07-11T00:30:15Z

Right now we have been adding authors to tutorials any time there is an update.

I don't think it needs to be displayed prominently in the example gallery, but I could imagine instances where people might want a receipt for their level of participation. It also provides a receipt for their level of expertise in Astropy.

Does simply relying on the github history provide a an accurate description of those items? I'm not sure.

I suggest it as an optional piece of meta-data because obviously a lot of the examples are already written and we may not have a record of who actually wrote them.

bsipocz · 2019-07-11T16:14:18Z

Yes, but it goes against what we have in the core package, including its documentation. There are no authors, its a community product where people are listed in the credits page alphabetically.
I'm not sure how comfortable I'm with the idea of people contribute for a receipt for their level of expertise rather than to improve something for the common good.

eblur · 2019-07-11T20:09:03Z

I don't care about this as much as you do, so let's forget about it. Sounds like this is a much deeper question than I intended it to be.

The purge_doc callback is required to remove examples from a cached environment if a document is removed in a subsequent build. Otherwise the examples in the cached environment from previous builds would continue to exist in subsequent builds. The tests simulate a env-purge-doc event and separately ensure that purge_examples got registered as a env-purge-doc callback.

This refactoring allows _check_for_existing_example to be used outside the ExampleMarkerDirective, like in a env-merge-info event callback.

This env-merge-info callback handles merging sphinx_astropy_examples from parallel build environments when Sphinx is run in parallel read mode. The tests run a full-scale integration-type build with Sphinx running in parallel (-j 4).

jonathansick · 2019-07-16T15:55:43Z

I've dealt with handling parallel builds and environment cleanup. I think the last thing to deal with is getting confirmation from @astrofrog on the "cases" versus "roots" terminology discussed in this thread.

I'm moving on to focusing on the other part of the example extension that actually generates the example gallery pages. I'm still working out how this will come together, but it may follow a pattern similar to automodapi in order to add additional pages to the build. In that case, some of the internal functionality in the example directive that's produced here might get replaced in order to work with that automodapi-like model. So we can either merge this PR so that doc contributors can start trying out the example markup in docs, or we can keep this PR in stasis while I work on the gallery part.

astrofrog · 2019-08-20T07:47:21Z

By the way, we now require Sphinx 1.7 and above in sphinx-automodapi so feel free to make that change here too and mention it in the changelog.

One question I forgot to ask before is how this interacts with the pytest plugin for rst docs - do the examples here get picked up as tests that should be run? (if not, we'll need to make sure the pytest-doctestplus plugin is updated so that this works).

At this point, I'm inclined to say that I think we should wait for the complete solution before merging since some of the implementation here might still change (I don't think we'd want to do a release of sphinx-astropy with only the partial implementation so people would need to install the developer version anyway to try it out, and at that point they could also just install it from your fork).

astrofrog · 2019-11-12T20:14:46Z

@jonathansick - just to check, what is the status of this?

jonathansick · 2019-11-12T23:05:09Z

@astrofrog Sorry I've forgotten to close this PR after we talked about delaying the merge until the merge is ready.

I've got a new PR in the works with the full extension (end-to-end extracting examples and publishing the example gallery). The draft PR in my fork is jonathansick#2

What I have left to do is:

Fix up a few remaining compatibility issues with Matplotlib plot directives
Write architecture documentation
Write user documentation

astrofrog · 2019-11-13T09:17:10Z

@jonathansick - thanks! I just left a quick comment on that PR regarding parallel processing, but I won't review the rest yet.

jonathansick added 9 commits June 28, 2019 11:35

Add example extension to v1 sphinx config

cd03b0c

This includes the sphinx_astropy.ext.example Sphinx extension by default in Sphinx builds.

Add an example-gallery test site

cc72d1f

The example-marker.rst file contains several instances of the example directive, testing different conditions (having tags, or not, and having different types of content in the example).

Test example directive's persistence to app env

16a7e05

Document example directive in change log

4421a43

kelle requested a review from astrofrog July 2, 2019 17:42

astrofrog requested changes Jul 4, 2019

View reviewed changes

jonathansick added 6 commits July 6, 2019 15:09

fixup! Add example extension to v1 sphinx config

7e7a776

fixup! Add ExampleMarkerDirective

3bc878c

Test for examples with duplicate titles

c877043

There is a new test case (test-example-gallery-duplicates) because otherwise the SphinxError would always be raised for regular testing of the example directive.

Test an example directive in numpydoc docstring

ccb8ba0

Now the test-example-gallery root is using an autodoc+numpydoc processing pipeline in its build configuration. This confirms that the directive does work in a docstring as expected.

fixup! Test an example directive in numpydoc docstring

779147e

kelle mentioned this pull request Jul 8, 2019

Should examples in docstrings be included in the example gallery? astropy/astropy-tutorials#378

Open

jonathansick added 4 commits July 12, 2019 10:41

Refactor check for existing examples

48295a2

This refactoring allows _check_for_existing_example to be used outside the ExampleMarkerDirective, like in a env-merge-info event callback.

Add merge_examples callback for parallel builds

a18fd9d

This env-merge-info callback handles merging sphinx_astropy_examples from parallel build environments when Sphinx is run in parallel read mode. The tests run a full-scale integration-type build with Sphinx running in parallel (-j 4).

fixup! Add merge_examples callback for parallel builds

8e7babd

jonathansick closed this Nov 12, 2019

jonathansick mentioned this pull request Dec 6, 2019

Add sphinx_astropy.ext.example extension for building an example gallery #29

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an example directive to mark in-text examples for the example gallery #22

Add an example directive to mark in-text examples for the example gallery #22

jonathansick commented Jun 30, 2019

bsipocz commented Jul 1, 2019

bsipocz commented Jul 1, 2019

jonathansick commented Jul 2, 2019

bsipocz commented Jul 2, 2019

adrn commented Jul 2, 2019

astrofrog left a comment

astrofrog Jul 4, 2019

astrofrog Jul 4, 2019

jonathansick Jul 6, 2019

astrofrog Aug 20, 2019

astrofrog Jul 4, 2019

jonathansick Jul 6, 2019

jonathansick commented Jul 6, 2019

bsipocz commented Jul 6, 2019

kelle commented Jul 8, 2019 •

edited

bsipocz commented Jul 8, 2019

jonathansick commented Jul 8, 2019 •

edited

eblur commented Jul 9, 2019 •

edited

jonathansick commented Jul 9, 2019

bsipocz commented Jul 10, 2019

eblur commented Jul 11, 2019

bsipocz commented Jul 11, 2019

eblur commented Jul 11, 2019

jonathansick commented Jul 16, 2019

astrofrog commented Aug 20, 2019

astrofrog commented Nov 12, 2019

jonathansick commented Nov 12, 2019

astrofrog commented Nov 13, 2019

Add an example directive to mark in-text examples for the example gallery #22

Add an example directive to mark in-text examples for the example gallery #22

Conversation

jonathansick commented Jun 30, 2019

Example metadata

Target node

Testing

bsipocz commented Jul 1, 2019

bsipocz commented Jul 1, 2019

jonathansick commented Jul 2, 2019

bsipocz commented Jul 2, 2019

adrn commented Jul 2, 2019

astrofrog left a comment

Choose a reason for hiding this comment

astrofrog Jul 4, 2019

Choose a reason for hiding this comment

astrofrog Jul 4, 2019

Choose a reason for hiding this comment

jonathansick Jul 6, 2019

Choose a reason for hiding this comment

astrofrog Aug 20, 2019

Choose a reason for hiding this comment

astrofrog Jul 4, 2019

Choose a reason for hiding this comment

jonathansick Jul 6, 2019

Choose a reason for hiding this comment

jonathansick commented Jul 6, 2019

bsipocz commented Jul 6, 2019

kelle commented Jul 8, 2019 • edited

bsipocz commented Jul 8, 2019

jonathansick commented Jul 8, 2019 • edited

eblur commented Jul 9, 2019 • edited

jonathansick commented Jul 9, 2019

bsipocz commented Jul 10, 2019

eblur commented Jul 11, 2019

bsipocz commented Jul 11, 2019

eblur commented Jul 11, 2019

jonathansick commented Jul 16, 2019

astrofrog commented Aug 20, 2019

astrofrog commented Nov 12, 2019

jonathansick commented Nov 12, 2019

astrofrog commented Nov 13, 2019

kelle commented Jul 8, 2019 •

edited

jonathansick commented Jul 8, 2019 •

edited

eblur commented Jul 9, 2019 •

edited