Opened 6 years ago

Closed 6 years ago

#11251 closed enhancement (fixed)

Add todo extension to Sphinx

Reported by: hivert Owned by: hivert
Priority: major Milestone: sage-4.7.2
Component: documentation Keywords: todo sphinx days30
Cc: leif, novoselt Merged in: sage-4.7.2.alpha0
Authors: Florent Hivert Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by hivert)

Sphinx has a todo extension which allows to highlight and gather them in a nice todo-list. I thinks it is worth adding it to sage doc. See also this thread on sage-devel.

Attachments (2)

trac_11251-sphinx_todo_extension-fh.patch (6.3 KB) - added by hivert 6 years ago.
trac_11251-referee.patch (1.3 KB) - added by jhpalmieri 6 years ago.
apply on top of other patch

Download all attachments as: .zip

Change History (26)

comment:1 Changed 6 years ago by hivert

  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 6 years ago by hivert

  • Status changed from needs_review to needs_work

I currently nee to commute it with my patch for #9128...

comment:3 Changed 6 years ago by hivert

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:4 follow-up: Changed 6 years ago by jhpalmieri

This looks pretty good to me. I would change "The To Do list of sage" -- at least capitalize Sage. Maybe there should also be a comment that the list is incomplete. (A "to do" item could be to add to the to do list :)

comment:5 in reply to: ↑ 4 Changed 6 years ago by hivert

Replying to jhpalmieri:

This looks pretty good to me. I would change "The To Do list of sage" -- at least capitalize Sage. Maybe there should also be a comment that the list is incomplete. (A "to do" item could be to add to the to do list :)

Done. One less "to do" item :)

comment:6 Changed 6 years ago by hivert

  • Owner changed from mvngu to hivert

I really don't understand the buildbot error. What the hell does it has to do with what I'm doing. Moreover I can't reproduce it on my laptop:

}}}popcorn-*stall/sage-4.6.2 $ sage -t  -force_lib devel/sagenb-main/sagenb/misc/misc.py
sage -t -force_lib "devel/sagenb-main/sagenb/misc/misc.py"  
         [2.5 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 2.6 seconds

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

Hello. I noticed that the todo comments in the command-line documentation do not appear. Here is a short example you can do on the command line:

sage: def f(): 
....:     r"""
....:     Hello!
....:     """
....:     pass
....: 
sage: 
sage: f?
Type:           function
Base Class:     <type 'function'>
String Form:    <function f at 0x110441f50>
Namespace:      Interactive
File:           /Users/saliola/Applications/sage-4.7.rc1/devel/sage-trac/sage/combinat/posets/<ipython console>
Definition:     f()
Docstring:
       Hello!


sage: def g():
....:     r"""
....:     .. todo::
....:         Hello
....:     """ 
....:     pass
....: 
sage: g?
Type:           function
Base Class:     <type 'function'>
String Form:    <function g at 0x10ef6b050>
Namespace:      Interactive
File:           /Users/saliola/Applications/sage-4.7.rc1/devel/sage-trac/sage/combinat/posets/<ipython console>
Definition:     g()

sage: 

comment:8 Changed 6 years ago by hivert

  • Keywords days30 added

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

Hi!

Any chance to have this finalized soon? Currently #11287 (runsnake) depends on it.

comment:10 in reply to: ↑ 9 Changed 6 years ago by hivert

  • Status changed from needs_review to needs_info

Any chance to have this finalized soon? Currently #11287 (runsnake) depends on it.

I've identified the problem: the file sage-4.7/devel/sagenb/sagenb/misc/sphinxify.py contains a non updated copy of the configuration file sage-4.7/devel/sage/doc/common/conf.py

So if I find out why we don't simply fetch the file, I might fix it very soon.

Florent

Changed 6 years ago by hivert

comment:11 in reply to: ↑ 7 Changed 6 years ago by hivert

  • Status changed from needs_info to needs_review

Hello. I noticed that the todo comments in the command-line documentation do not appear. Here is a short example you can do on the command line:

This should now be fixed. Back to needs review.

comment:12 Changed 6 years ago by jhpalmieri

This looks pretty good to me. I'm attaching a small referee patch. Should we also patch sphinxify in the sagenb repo?

Changed 6 years ago by jhpalmieri

apply on top of other patch

comment:13 Changed 6 years ago by nthiery

Positive review on the reviewer's patch. John: you may put a positive review if you feel like it now!

comment:14 follow-up: Changed 6 years ago by jhpalmieri

Well, does sphinxify.py need to be patched?

comment:15 in reply to: ↑ 14 Changed 6 years ago by hivert

Replying to jhpalmieri:

Well, does sphinxify.py need to be patched?

Sorry for the confusion. My analysis of the problem was wrong. There is a code duplication and the actual file I had to patch is

sage-4.7/devel/sage-combinat/doc/en/introspect/conf.py

You can double check that it works both under the command line and the notebook. I actually don't really know what this sphinxify.py is here.

Cheers,

Florent

comment:16 Changed 6 years ago by jhpalmieri

  • Reviewers set to John Palmieri
  • Status changed from needs_review to positive_review

Introspection works, but isn't perfect. I think it's good enough for now, but it might be nice to try to figure out what's wrong eventually. In particular: look at a docstring with a ".. todo" markup in these four ways:

  • in the reference manual: it looks great. "TODO" is in bold, and the whole block has a different background color.
  • in the notebook (by typing sage.combinat.combinat?, for example): it looks okay. "TODO" is not in bold, and the block has the same background color as the rest of the docstring.
  • from the command line, using ...?: it looks as good as can be expected, since it's plain text.
  • from the command line, using browse_sage_doc(sage.combinat.combinat): it looks pretty good, in between the reference manual and the notebook: "TODO" is in bold, but the background color is white.

I tried modifying sphinxify.py and it didn't seem to have any effect. If only there were some way to add something like this to a "to do list"...

comment:17 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-4.7.1 to sage-4.7.2

comment:18 Changed 6 years ago by leif

  • Cc leif added

comment:19 follow-ups: Changed 6 years ago by leif

Maybe I misunderstood the intent, but is there a way to omit the TODOs from "production" documentation?

(I thought they were only for "internal" use, i.e. invisible to an "ordinary" user, as opposed to a developer working on the documentation, similar to \note{} of LaTeX's "beamer" class.)

comment:20 in reply to: ↑ 19 Changed 6 years ago by hivert

Replying to leif:

Maybe I misunderstood the intent, but is there a way to omit the TODOs from "production" documentation?

(I thought they were only for "internal" use, i.e. invisible to an "ordinary" user, as opposed to a developer working on the documentation, similar to \note{} of LaTeX's "beamer" class.)

Actually, this wasn't my intend. As an open source project, I don't see the point in hiding places where sage could be improved. On the contrary, marking clearly in the documentation: "there is something we want to improve here" seems to me to be a good practice. This was my main motivation for adding this extension.

Otherwise said, in my mind those TODOs are more for long term improvement where we don't know yet what to do and some feedback is needed or where we don't have yet the manpower to do it, But my point of view is maybe not shared by the majority of sage developers/users.

Now to answer the technical question, in doc/common/conf.py you have

# include the todos
todo_include_todos = True

Changing it to False by hand or depending on an environment variable should do the job. Still, I'd rather having it to True by default.

Florent

comment:21 in reply to: ↑ 19 Changed 6 years ago by jhpalmieri

Replying to leif:

Maybe I misunderstood the intent, but is there a way to omit the TODOs from "production" documentation?

(I thought they were only for "internal" use, i.e. invisible to an "ordinary" user, as opposed to a developer working on the documentation, similar to \note{} of LaTeX's "beamer" class.)

I understood this markup is used to mark places where the code needs improvement, not the documentation. Also, as you know, with Sage the distinction between users and developers is intentionally blurred.

comment:22 Changed 6 years ago by jhpalmieri

See #6495 for a followup, of sorts. That ticket breaks the reference manual into chunks and builds them in parallel, which can greatly speed things up, but I can't figure out a good way to collect all of the todo items into a single list. So right now, the patch makes each Sage module collect its own todo items. Please take a look and suggest improvements.

comment:23 Changed 6 years ago by novoselt

  • Cc novoselt added

comment:24 Changed 6 years ago by jdemeyer

  • Merged in set to sage-4.7.2.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.