chapel-lang / pychapel

pych - The Python/Chapel integration module. NOTE: This repository is now deprecated.
Apache License 2.0
16 stars 13 forks source link

docs: Make line-limited literalincludes less fragile #74

Closed cassella closed 6 years ago

cassella commented 7 years ago

docs: Make line-limited literalincludes less fragile

Particularly, harden them against code motion.

Use :end-before: instead of line numbers to stop the full program listings before the test routines. To this end, add a sentinel comment line to the python programs where the literalinclude should end.

Use the :pyobject: line-selection helper in the one case it applies -- the pure python quant(). It can't be used for the Chapel version because it won't include the leading @Chapel() with our current pinned version of sphinx 1.3.6; fixed by https://github.com/sphinx-doc/sphinx/issues/3348

The test_finance_chapel_numpy.py file that includes the @Chapel() quant is not included as a full file anywhere, only for this one function and an import line, so use sentinel lines for those too.

cassella commented 7 years ago

There are more includes in usage.rst that this approach could apply to, but I wanted to see if the idea passes muster first.

lydia-duncan commented 7 years ago

I'm in favor of having the line numbers grabbed be less fragile, and this seems like a reasonable approach to me. Does "begin-after" also base itself on a tag?

cassella commented 7 years ago

It does. I found that and the pyobject directive in http://www.sphinx-doc.org/en/stable/markup/code.html

My first attempt at this used start-after and end-before for all the bits of code inclusion. But then the literalincludes that include nearly the whole file also include all those sentinel tags in-line. :frowning_face:

Maybe with more effort, one could come up with sentinel tags that fit naturally in the file.

I'm open to suggestion for a better tag to use -- that one was just the first thing that came to mind as not going to be elsewhere in the file, and self-explanatory to the reader and editor of the actual .py files.

lydia-duncan commented 7 years ago

What about having the start tag be the start of a documentation-style comment for the function? That way it fits in naturally in the whole file inclusion. The end section could be a "end of foo's definition" or something to that effect, that you might see normally on a longer function.

Of course, this would mean these tags vary from file to file, which could get a bit tedious.

cassella commented 7 years ago

That makes sense. The tags would need to vary anyway in order to include two separate bits from the same file.

cassella commented 7 years ago

Though on the other hand, since it's start-after, the line in the intro comment that's used as the tag won't itself be included in the targeted include. And it seems like the tag string would need to make it obvious to passers-by that it's being used externally, so please don't rephrase it without changing the docs too...

I wonder if the sphinx project would be amenable to having the pyobject directive include decorators like @Chapel().

lydia-duncan commented 7 years ago

Hmm, true. There's probably a balance between "this looks too out of place to include in the export of the code" and "this is too subtle and so is vulnerable to tweaking". Perhaps one involving some checks to make sure that the tag is always present in the file? That might be something the Travis test should run . . .

cassella commented 7 years ago

Ah, the .travis.yml already has a "cd docs/ && make html". I believe I've already, err, tested, that that fails when the tag isn't found.

lydia-duncan commented 7 years ago

I love it when we've already taken care of this stuff! Alright, how about you follow that path and get that all taken care of in this PR?

cassella commented 7 years ago

Ah, they were amenable to it in March: https://github.com/sphinx-doc/sphinx/issues/3348

cassella commented 7 years ago

Ooops, nope. If the start-after tag isn't found, nothing is included. If the end-before tag isn't found, it includes through the end of the file.

I'd thought that had caused the build to fail. Maybe I'd only said FAIL so loudly it seemed official.

I'm not sure what a Travis check for that case would look like. Most of the examples/ files wouldn't have the tags, and the tags wouldn't necessarily be the same in all of them. So I think it would either need to parse the literalinclude:: sections of the .rsts; or have its own metadata listing what files should have what tags, which would need to be kept up to date with the .rst files.

Unless there's a way to invoke sphinx such that it's stricter about this sort of thing? I can't find one via google or sphinx-build -h or the man page.

cassella commented 7 years ago

Cleaned it up a bit and squashed, as nearly every line in the original commit was changed.

cassella commented 7 years ago

Of note, since test_finance_chapel_numpy.py was included only in snippets, I used tags for the include of the pyChapel version of quant() without having to worry about how they'd look when included.

lydia-duncan commented 7 years ago

This seems relevant: https://github.com/sphinx-doc/sphinx/issues/3108

lydia-duncan commented 7 years ago

We would need to update our Sphinx version to get that fix, which I can look into

lydia-duncan commented 7 years ago

Wow, this lounged for a while. Can you remind me why we didn't merge it yet?

cassella commented 7 years ago

I believe it was that the version of sphinx that the pychapel build uses won't report an error if the start-after or end-before tags don't match anything. The sphinx-doc issue you pointed out a few comments up covered adding that error reporting, so that the build will fail if those sentinel lines in the source are modified without updating the literalincludes in the .rst files.

And there's a good chance that once pychapel uses a sphinx with that feature, that new sphinx will also include the feature of the pyobject directive including the @Chapel decorator, so that the pyChapel quant case can use that instead. In that case I can update the PR before it's merged.

Assuming that the readthedocs build of the docs uses the new sphinx with the pyobject decorator support. I wonder if they've gotten the fixed line numbering layout yet.

lydia-duncan commented 6 years ago

Pychapel has been abandoned, closing (shoulda done this earlier)