Opened 7 years ago

Closed 5 years ago

#14512 closed enhancement (fixed)

Use ....: doctest continuations in documentation

Reported by: jdemeyer Owned by: mvngu
Priority: trivial Milestone: sage-6.4
Component: documentation Keywords:
Cc: roed, zimmerma, kcrisman Merged in:
Authors: Jeroen Demeyer Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: e55c9e7 (Commits) Commit: e55c9e7919a5161463ce4498d0dbd953225b8d3d
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

Automatically replace

sage:
...
...

by

sage:
....:
....:

in all documentation files, since those are changed rarely and we should certainly teach the right thing there. The patch also adds some manual fixes for doctest formatting, including the removal of trailing backslashes in most places.

Attachments (1)

doctest_prompt.sh (532 bytes) - added by jdemeyer 7 years ago.
script to generate the patch

Download all attachments as: .zip

Change History (34)

comment:1 Changed 7 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 7 years ago by jdemeyer

  • Dependencies set to #11745 #12894 #13698 #13961 #14252 #14287 #14299 #14424 #14461 #14477 #14479 #14483 #8703

comment:3 follow-up: Changed 7 years ago by vbraun

How about

  • spell out in the developer guide how to write line continuations
  • disallow the old notation

Otherwise people will just keep using the old-style line contiuations.

comment:4 in reply to: ↑ 3 Changed 7 years ago by jdemeyer

Replying to vbraun:

How about

  • spell out in the developer guide how to write line continuations

This is already done (although not yet in the online documentation, which is for Sage 5.8).

  • disallow the old notation

Maybe eventually we should do this (and then we could also allow doctests whose expected output starts with ...). But I think it's better to allow for a period where ... still works but ....: is encouraged.

Changed 7 years ago by jdemeyer

script to generate the patch

comment:5 Changed 7 years ago by jdemeyer

  • Status changed from new to needs_review

comment:6 follow-up: Changed 7 years ago by nthiery

Ouch ... This is likely to be a pain and create lots of conflicts ... Is it really worth the trouble?

Well, luckily it should not be too hard to rebase the patches, and it might get less noticed in the middle of the change of workflow. But still ...

comment:7 Changed 7 years ago by vbraun

We should do it at the same time as the trailing-whitespace patch, I think. People won't know what hit them ;-)

comment:8 in reply to: ↑ 6 ; follow-up: Changed 7 years ago by jdemeyer

Replying to nthiery:

Ouch ... This is likely to be a pain and create lots of conflicts ... Is it really worth the trouble?

I think it would be good to change these doctests. If we disallow the ... syntax, then we could finally write doctests with expected output starting with ellipsis ....

Well, luckily it should not be too hard to rebase the patches, and it might get less noticed in the middle of the change of workflow.

With some small modifications, the script on this ticket could be used to auto-rebase patches. I could run this script when merging patches and I could also run it on the sage-combinat queue for example. I think that should ease most of the pain.

comment:9 in reply to: ↑ 8 Changed 7 years ago by nthiery

Replying to jdemeyer:

Replying to nthiery:

Ouch ... This is likely to be a pain and create lots of conflicts ... Is it really worth the trouble?

I think it would be good to change these doctests. If we disallow the ... syntax, then we could finally write doctests with expected output starting with ellipsis ....

Well, I very much hope the "..." syntax won't be disallowed right away: people have a *lot* of .sage files out there which include doctests. Dropping backward compatibility will force them to update them at once. Which means they won't be able to share them properly with other people running older versions of Sage (ok, the files are still usable, but one won't be able to run the tests; for example to make sure that everything works fine with all versions in use).

There are also printed or about to be printed *books* that have been written with this syntax and sagetex. It's useful to be able to test the examples in those books on various versions of Sage over a non trivial amount of time.

I appreciate the gain of tests starting with "...:. All our "Traceback ..." lines are kind of silly. But it's really not critical either. We lived without it for years, we could wait a bit more.

Well, luckily it should not be too hard to rebase the patches, and it might get less noticed in the middle of the change of workflow.

With some small modifications, the script on this ticket could be used to auto-rebase patches. I could run this script when merging patches and I could also run it on the sage-combinat queue for example. I think that should ease most of the pain.

I appreciate the offer! Please take into account that, once rebased, the patch won't apply anymore on "older" versions of Sage. Well, unless you duplicate the patch in the queue, with guards, and all the usual pain of duplication for things that are still evolving.

So as much I could be ok with this change occurring right at the time the Sage-Combinat queue gets reborn as a git tree (with e.g. the patches being auto-rebased when they are imported into git). As much I would oppose a change before that. I guess that means not in any 5.*.

There will be a lot of turmoil ahead for us to handle the transition. Please don't add yet another hurdle!

Cheers,

Nicolas

comment:10 follow-up: Changed 7 years ago by jdemeyer

Nicolas, I actually totally understood your point.

How do you feel about automatically converting all new/changed doctests to the new format (say, with one patch per beta to do that)?

comment:11 in reply to: ↑ 10 ; follow-ups: Changed 7 years ago by nthiery

Replying to jdemeyer:

Nicolas, I actually totally understood your point.

How do you feel about automatically converting all new/changed doctests to the new format (say, with one patch per beta to do that)?

Way to go! Or even requesting that any new/changed doctest should use the new syntax right away, like we have been doing for whitespaces and other things.

And hopefuly, once we have switched to the new workflow, merging/rebasing will be much smoother. In that case, large scale changes like the above could become reasonable (which would be good!).

Speaking of which: one interesting tool is http://coccinelle.lip6.fr/. It allow to describe such changes using "semantic" meta-patches, which are much more context free and could make merges much easier. I haven't tried it myself.

comment:12 in reply to: ↑ 11 Changed 7 years ago by jdemeyer

Replying to nthiery:

How do you feel about automatically converting all new/changed doctests to the new format (say, with one patch per beta to do that)?

Way to go! Or even requesting that any new/changed doctest should use the new syntax right away, like we have been doing for whitespaces and other things.

Well, we're not forcing anything for whitespace, we're just suggesting...

comment:13 in reply to: ↑ 11 Changed 7 years ago by jdemeyer

Replying to nthiery:

Speaking of which: one interesting tool is http://coccinelle.lip6.fr/. It allow to describe such changes using "semantic" meta-patches, which are much more context free and could make merges much easier.

From what I have seen, it looks like a good tool to generate a patch like on this ticket, but it doesn't seem to solve to problem of rebasing other patches.

comment:14 Changed 7 years ago by nthiery

From the talk I had seen some time ago, there was some interesting "patch" arithmetic that could be done using the semantic. However I haven't followed up to see what's actually implemented / available.

comment:15 Changed 7 years ago by ncohen

sooooo what happens with this patch ? Does it still need a review ?

Nathann

comment:16 follow-up: Changed 6 years ago by kcrisman

See my recent message on sage-devel about this; as far as I can tell, this breaks the live documentation. I haven't tried the patch here, this is from experimentation on #13381.

comment:17 Changed 6 years ago by kcrisman

This has been confirmed on the same thread.

comment:18 Changed 6 years ago by kini

It would be nice if you could also check if the indentation level % 4 = 2 and remove two spaces after the : if so.

comment:19 Changed 6 years ago by kini

Actually, a version of what I requested which is guaranteed not to break even weird looking doctests probably is nontrivial. Never mind, I guess. I guess it's not so terrible to have doctests that look like this:

sage: def f():
....:       if 0 == 0:
....:           return false
....:       return true

comment:20 in reply to: ↑ 16 Changed 6 years ago by kcrisman

  • Dependencies changed from #11745 #12894 #13698 #13961 #14252 #14287 #14299 #14424 #14461 #14477 #14479 #14483 #8703 to #11745 #12894 #13698 #13961 #14252 #14287 #14299 #14424 #14461 #14477 #14479 #14483 #8703 #14330

See my recent message on sage-devel about this; as far as I can tell, this breaks the live documentation. I haven't tried the patch here, this is from experimentation on #13381.

Okay, this seems to be fixed here, so I'm making this ticket depend on the sagenb upgrade.

comment:21 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:22 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:23 Changed 6 years ago by pbruin

  • Status changed from needs_review to needs_work

The patch must be outdated by now, and the script is still based on Mercurial...

comment:24 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:25 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:26 Changed 5 years ago by jdemeyer

  • Component changed from doctest coverage to documentation
  • Dependencies #11745 #12894 #13698 #13961 #14252 #14287 #14299 #14424 #14461 #14477 #14479 #14483 #8703 #14330 deleted
  • Description modified (diff)
  • Summary changed from Use ....: doctest continuations everywhere to Use ....: doctest continuations in documentation

I propose to do this just for documentation.

Last edited 5 years ago by jdemeyer (previous) (diff)

comment:27 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/14512
  • Created changed from 05/01/13 08:26:36 to 05/01/13 08:26:36
  • Modified changed from 11/23/14 20:21:28 to 11/23/14 20:21:28

comment:28 Changed 5 years ago by jdemeyer

  • Cc kcrisman added
  • Commit set to 8a06f3bbb0020c75490074e14bb7f6d6fd7cc161
  • Description modified (diff)
  • Status changed from needs_work to needs_review

New commits:

8a06f3bFix doctest formatting in src/doc

comment:29 Changed 5 years ago by git

  • Commit changed from 8a06f3bbb0020c75490074e14bb7f6d6fd7cc161 to e55c9e7919a5161463ce4498d0dbd953225b8d3d

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

e55c9e7Fix doctest formatting in src/doc

comment:30 follow-up: Changed 5 years ago by vbraun

ready for review now?

comment:31 in reply to: ↑ 30 Changed 5 years ago by jdemeyer

Replying to vbraun:

ready for review now?

Yes.

comment:32 Changed 5 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

comment:33 Changed 5 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/14512 to e55c9e7919a5161463ce4498d0dbd953225b8d3d
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.