Skip to content
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

Reorganize the documentation indexes into src/sage/combinat #16256

Closed
nthiery opened this issue Apr 28, 2014 · 177 comments
Closed

Reorganize the documentation indexes into src/sage/combinat #16256

nthiery opened this issue Apr 28, 2014 · 177 comments

Comments

@nthiery
Copy link
Contributor

nthiery commented Apr 28, 2014

Goal: reorganize the documentation indexes into src/sage/combinat

  • For example, the thematic index
    src/doc/en/reference/combinat/crystals.rst is now in:
    src/sage/combinat/crystals/__init__.py and is accessible through
    sage.combinat.crystals?

    (potential variant: put it in all.py)

  • What's left in doc/en/reference/combinat can potentially be generated automatically.
    This is not yet automatized; the content of module_list.rst still needs to be updated by hand; see the instructions on top of the file.

  • All p/cython files in src/sage/combinat/ are now included in the reference manual

  • Improved thematic indexes

  • New thematic indexes: algebraic_combinatorics, catalog_partitions,
    counting, enumerated_sets

  • Fixed some documentation syntax glitches here and there

  • Added the catalogs of permutation groups and matrix groups to the
    reference manual so that we can link to them. Fixed the doc title of
    the former.

  • Added a back link from the doc of sage.dynamics

  • Added (draft of) sage.combinat.quickref

This is a follow up to #16058.

End result browsable at ​http://sage.math.washington.edu/home/nthiery/sage/src/doc/output/html/en/reference/combinat/index.html

Warning: see the note below about the current doc compilation glitch.

Discussion

One might argue that this reorganization of the documentation is not
consistent with what's done elsewhere in the manual. Indeed. I believe
sage.combinat is a good spot to explore better ways to organize the
documentation. Here are the advantages of this new organization:

  • It's more local:

    E.g. everything the developer has to look at or modify about
    designs, including the index, is in src/sage/combinat/design. This
    is simpler for newcomers and means, e. g., less chances to forget to
    update the index w.r.t. the code.

  • It's more consistent with the hierarchical structure of modules. In
    particular, it's agnostic of how the reference manual is split into
    documents. This was not the case before: to split
    sage/combinat/crystals in its own document required to move the
    thematic index about crystals from reference/combinat/ to
    reference/combinat/crystals. This will ease the job of Reorganization of combinatorics documentation #14582.

  • It's more friendly to documentation lookup using introspection

  • It's more automatic; there are less chances to forget adding a file
    in the documentation which hit us often in the past.

  • It enforces including all files in the documentation.

  • Writing the thematic indexes as plain lists (rather than toctrees)
    is more flexible:

    • one can e.g. choose to point to the main class in a file rather
      than the full file.

    • one can focus on the user feature and not reference technical
      modules (they appear in the full module list anyway)

    • one can point to related things elsewhere in the source code

TODO

  • Proof reading

  • Checking that the links are functional

  • Handle the various TODO's left in the indexes (typically about
    choosing the right entry points for each module)

    Postponed to a later ticket, since those are further enhancements
    not directly related to this ticket. It's just that, while browsing
    through the indexes suggested them to me.

  • Finish automatizing the building of module_list.rst.

    Postponed to Automatic generation of the module list in the Sphinx documentation for sage.combinat #17421

  • Sphinx issue: deciding on how to link to classes/functions

    in the index we would want to have the title of the documentation of
    the class rather than the name of the class; or maybe both.

    For now, we stick to the class name for now until we find a good way
    to do this with Sphinx.

  • Sphinx issue: how to crossrefs documents in the thematic tutorial.

    For now we use a workaround.

  • Sphinx issue: homogeneous styles for the index links

    The style used by Sphinx depends on the type of the link; this makes
    the indexes look non homogeneous. See what we can do here. This is
    purely about style rather than content, hence postponed to a later
    ticket.

  • Sphinx issue: latex support in :ref:

    When the title of a module contains some latex, it gets displayed
    properly in tocrefs, but not in :ref's. We had a look with
    Florent, and fixing this would require some fiddling with Sphinx's
    internals (the latex chunks are already stripped away in the
    crossref database!). Hence postponed for later as fixing this won't
    require changing the documentation sources.

Depends on #16058
Depends on #17189

CC: @sagetrac-sage-combinat @nathanncohen @tscrim @anneschilling @videlec

Component: documentation

Keywords: thematic index, quickref

Author: Nicolas M. Thiéry, Jean-Pierre Flori

Branch: 823c116

Reviewer: Anne Schilling, Nathann Cohen

Issue created by migration from https://trac.sagemath.org/ticket/16256

@nthiery nthiery added this to the sage-6.2 milestone Apr 28, 2014
@nthiery

This comment has been minimized.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Apr 28, 2014

Commit: 391f7ff

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Apr 28, 2014

Branch: u/nthiery/16058-combinat-doc-index

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Apr 28, 2014

New commits:

8b8aea8trac #16058: Organize the index of combinatorial modules
e0d2b66trac #16058: Two new categories
0293c49trac #16058: Another group
82af706Merge branch 't/16100/keep_going_in_doc_errors' into t/16058/public/16058
28062edExperiment with moving the indexes around and toward the source tree
63b43d4Merge 6.2.beta8 into t/16058/public/16058
499aae1Trac 16058: change the label in the sphinx-autodoc files for .__init__.py to
6755031Trac 16058: Reorganize the documentation indexes into src/sage/combinat
3b84036Trac 16058: Reorganize the documentation indexes into src/sage/combinat (follow up little fixes)
391f7ffMerge branch 'develop' into t/16058/public/16058

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Apr 28, 2014

comment:3

(btw : you can check that the links are functional with the -k --warn-links options : sage -docbuild reference/combinat -k --warn-links html)

@nthiery
Copy link
Contributor Author

nthiery commented Apr 28, 2014

Changed branch from u/nthiery/16058-combinat-doc-index to none

@nthiery
Copy link
Contributor Author

nthiery commented Apr 28, 2014

Changed commit from 391f7ff to none

@nthiery
Copy link
Contributor Author

nthiery commented Apr 28, 2014

comment:4

Replying to @nathanncohen:

  1. I have no idea how I can obtain the result from your web page with the branch you give.

Nothing special: checkout the branch and run make doc as usual.

By the way, it does not apply on the latest rc0.

I merged the trivial conflict.

  1. Look at that : http://sage.math.washington.edu/home/nthiery/16058-doc/combinat/sage/combinat/counting.html#sage-combinat-counting
    Or that : http://sage.math.washington.edu/home/nthiery/16058-doc/combinat/sage/combinat/species/__init__.html#sage-combinat-species

All links are broken!

Thanks for spotting this. I had fumbled my query replace. It should be
fixed now. Well, I am recompiling Sage right now. I'll push / update
the web page shortly.

That's a problem for a reference manual ...

No, really? Never thought about that :-)

  1. Where are the combinatorial designs ?

Ah, yes, it somehow slipped out of the main index. Fixed. Thanks.

  1. Those TODO will have to be removed before it is merged anywhere.
    Nicolas, it looks like your branch is not ready.

Of course.

Could you review this ticket and create another one for yours?

Will do / done.

You would also need to ask the release manager what he thinks of the script you have to run before generating the doc.

There is no such script to be run.

At this point, when there is a new module to be added, you can edit by
hand module_list.rst (as we used to do in index.rst). Or just be lazy,
and use the shell commands listed there. The point is that this step
can be automatized and is meant to be so.

Perhaps there is a pure-sphinx workaround ? I guess you asked Florent about this too ?

Yes, the plan is certainly to have module_list.rst be built
automatically by sphinx. Something similar to the building of the
other sphinx autogenerated files (in
reference/combinat/sage/combinat/*).

Right now I have no idea how it works. If you create another ticket, please explain that.

The only trick is in src/doc/en/reference/combinat/index.rst which
uses automodule to slurp in the documentation of sage.combinat which
itself is defined in sage/combinat/__init__.py

Other than that, this is just using standard ReST documentation.

@nthiery
Copy link
Contributor Author

nthiery commented Apr 28, 2014

comment:5

WARNING: there will be some non trivial conflicts with the latest changes in #16058.
The easiest for me to resolve them is to replay my changes on top of #16058 and create a new clean branch which I am going to work on now. The branch u/nthiery/16058-combinat-doc-index is just here for display; don't work on top of it!

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Apr 28, 2014

comment:6

?....

You can merge this branch with the head of the branch #16058, too... That only produces one additional commit and no history is rewritten.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Apr 28, 2014

comment:7

Ah, yes, it somehow slipped out of the main index. Fixed. Thanks.

Can you be sure it is the only one ?

You would also need to ask the release manager what he thinks of the script you have to run before generating the doc.

There is no such script to be run.

What is the meaning of this ?

+.. NOTE::
+
+   This is built automatically by running in src/sage/combinat::
+
+        for x in **/*.py*; do echo "    sage/combinat/"`echo $x | sed 's/\.py.?$//'`; done >! /tmp/module_list

At this point, when there is a new module to be added, you can edit by
hand module_list.rst (as we used to do in index.rst). Or just be lazy,
and use the shell commands listed there. The point is that this step
can be automatized and is meant to be so.

Okay, whatever. If the addition above is not about a script that has to be run before generating the doc, it has nothing to do there.

Yes, the plan is certainly to have module_list.rst be built
automatically by sphinx. Something similar to the building of the
other sphinx autogenerated files (in
reference/combinat/sage/combinat/*).

What is the point of having an index like that instead of the human-made index page, i.e. the one that is being worked on on this ticket ?

Nathann

@nthiery
Copy link
Contributor Author

nthiery commented Apr 28, 2014

comment:8

Replying to @nthiery:

I'll push / update the web page shortly.

Done. I pushed the branch too. Now working on making a clean branch.

@nthiery
Copy link
Contributor Author

nthiery commented Apr 28, 2014

comment:9

Replying to @nathanncohen:

Can you be sure it's the only one?

I don't have a formal proof, no :-) We will have to double check this
systematically. Like for any other reorganization.

Okay, whatever. If the addition above is not about a script that has to be run before generating the doc, it has nothing to do there.

This is documentation on how to update the module_list file; the top
of this file is a natural spot for it. In any cases this is meant to
be replaced by "automatically generated file; don't touch", so there
is no point discussing it.

What is the point of having an index like that instead of the human-made index page, i.e. the one that is being worked on on this ticket ?

For the reader that's indeed essentially pointless: sphinx already
provides an index of all documented modules. However sphinx currently
needs to have somewhere a list of all the modules for which it's
supposed to build the documentation, in the form of a toctree. Other
than that, I'd be more than happy to simply get rid of it.

@nthiery
Copy link
Contributor Author

nthiery commented Apr 28, 2014

@nthiery
Copy link
Contributor Author

nthiery commented Apr 28, 2014

comment:11

Pfiou, when sphinx goes into bad mode, this can be a pain to fix ... Oh well.

#16256 is now "on top" of #16058. I ended up doing it via a merge (at the end, it was no simpler than the replay I was planning to do which would have produced a cleaner history). Anyway, it's done, and the merge was the occasion to double check that everything that was in the previous index.rst is still referenced somewhere.

At this point this branch is not really ready for review, but wide open for comments.

The produced documentation has been updated.

Switching now to the review of #16058.


Last 10 new commits:

63b43d4Merge 6.2.beta8 into t/16058/public/16058
499aae1Trac 16058: change the label in the sphinx-autodoc files for .__init__.py to
6755031Trac 16058: Reorganize the documentation indexes into src/sage/combinat
3b84036Trac 16058: Reorganize the documentation indexes into src/sage/combinat (follow up little fixes)
391f7ffMerge branch 'develop' into t/16058/public/16058
ff7add9Fixed cross references, add title to all.py files
a14057bSome more groupings and separated root system types into separate list.
4b5cb2btrac #16058: Rebase on 6.2.rc0
4644c74Trac 16256: Merge branch 'public/16058' into u/nthiery/16058-combinat-doc-index
d328407Trac 16256: fixed some cross references and doc compilation issues

@nthiery
Copy link
Contributor Author

nthiery commented Apr 28, 2014

Commit: d328407

@nthiery
Copy link
Contributor Author

nthiery commented Apr 28, 2014

Work Issues: gather comments by showing it around, and work on the listed TODO's

@nthiery

This comment has been minimized.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@jpflori
Copy link

jpflori commented Jul 17, 2014

comment:16

Hey all,

What needs to be done here?
Building the combinat doc is one of the bottleneck on my machines, so this ticket is highly wanted.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jul 17, 2014

comment:17

What needs to be done here?
Building the combinat doc is one of the bottleneck on my machines, so this ticket is highly wanted.

If I understand correctly what this ticket is about, it rather goes against your goal, as it means to add more files to the documentation. But is it mostly about improving the doc to make it easier to read/browse through.

On the bright side, this ticket intends to do so many things requiring the input of so many persons that you can safely assume it will ... take time before anything is done :-P

On the other hand, beware if people start creating small tickets to address independently the points mentionned above, for that's how actual work begins :-P

Nathann

@jpflori
Copy link

jpflori commented Jul 17, 2014

comment:18

I thought this ticket also split the combinat doc into a bunch of subfolders, letting it be built in parallel.
Am I wrong?

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jul 17, 2014

comment:19

Yo !

I thought this ticket also split the combinat doc into a bunch of subfolders, letting it be built in parallel.
Am I wrong?

Well, I understood that some new index.rst files would be created, but I have no idea if this means that they will correspond to different entry points for sphinx and thus that they will be built separately O_o

This being said, if there is a way to achieve that you should probably look into Nicolas's branch to see if it is implemented already, and in any case create a second ticket to address it. Otherwise it will be held forever by the other things enumerated above.

Nathann

@vbraun
Copy link
Member

vbraun commented Dec 3, 2014

comment:130

Also conflicts with #16018 which I just merged.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Dec 4, 2014

Changed reviewer from Anne Schilling, Nathann Cohen to Anne Schilling

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Dec 4, 2014

comment:131

Just removing my name from the list of reviewer. Reviewing a ticket like that requires a LOT of time and careful checking, and I only looked at the couple of files that are of interest to me.

Nathann

@kcrisman
Copy link
Member

kcrisman commented Dec 4, 2014

Changed reviewer from Anne Schilling to Anne Schilling, Nathann Cohen

@kcrisman
Copy link
Member

kcrisman commented Dec 4, 2014

comment:132

Usually we tend to add names of anyone who did even partial review, as long as that is clear in the Trac comments. Of course you can once again remove your name, but there is no reason not to consider this a "contribution" unless you really feel strongly you shouldn't be on this reviewer list (which is fine, if so).

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Dec 4, 2014

comment:133

Yo !

Usually we tend to add names of anyone who did even partial review, as long as that is clear in the Trac comments.

Ahahah. Well, the comment I made when removing my name makes it rather clear :-)

Nathann

@jpflori
Copy link

jpflori commented Dec 8, 2014

comment:134

The conflict was trivial to fix.
The doctest issue as well (part of a test requiring internet was not marked so).


New commits:

70c677cMerge remote-tracking branch 'trac/develop' into ticket/16256
823c116Mark test needing internet as optional.

@jpflori
Copy link

jpflori commented Dec 8, 2014

Changed branch from u/nthiery/ticket/16256 to u/jpflori/ticket/16256

@jpflori
Copy link

jpflori commented Dec 8, 2014

Changed commit from 7fe79fb to 823c116

@nthiery
Copy link
Contributor Author

nthiery commented Dec 8, 2014

comment:135

I double checked the changes and confirm the positive review. Thanks Jean-Pierre for taking this task off my todo list of the day! I was running late :-)

@vbraun
Copy link
Member

vbraun commented Dec 12, 2014

Changed branch from u/jpflori/ticket/16256 to 823c116

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jan 2, 2015

comment:137

Hello everybody,

I am trying to add a document to the combinat/designs/ folder, and after adding a reference toward it in the __init__ file all I can get when I recompile the doc is this:

OSError: [combinat ] /home/ncohen/.Sage/local/lib/python2.7/site-packages/sage/combinat/designs/__init__.py:docstring of sage.combinat.designs.__init__:17: WARNING: undefined label: sage.combinat.designs.resolvable_bibd (if the link has no caption the label must precede a section header)

Is there something specific that must be done with the new system in order to add a new document ?

Thanks,

Nathann

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jan 2, 2015

Changed commit from 823c116 to none

@nthiery
Copy link
Contributor Author

nthiery commented Jan 2, 2015

comment:138

At this point, one still needs to add the file in src/doc/en/reference/combinat/module_list.rst.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jan 3, 2015

comment:139

At this point, one still needs to add the file in src/doc/en/reference/combinat/module_list.rst.

I see. I did not know that, and I tried many things yesterday during something like 30 minutes. I expect that I will not be the only one to meet such a problem in the future. Is there anything that you could do to avoid this problem ? Some way to bring this information to the attention of those who need it ? Without it this new combinat-specific management of documentation is bound to cause many headaches and much more lost time.

Nathann

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jan 6, 2015

comment:140

It is an unfortunate coincidence, but in the developer manual the combinat/ folder is used as an example of how to add a new file to the index, in the way that it had to be done before this patch was merged.

http://www.sagemath.org/doc/developer/sage_manuals.html#adding-a-new-file

Could you please take the time to update it, and to provide instruction about how this branch changed things in the combinat/ folder only ?

Thank you,

Nathann

@nthiery
Copy link
Contributor Author

nthiery commented Jan 6, 2015

comment:141

Thanks for the report.

I guess it would be sufficient to just add a comment in .../combinat/index.rst stating that, at this point, new modules should be added to .../module_list.rst instead.

I won't get to do it before the next two weeks. I am happy to review it if someone finds it urgent enough to beat me to it.

Cheers,

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jan 16, 2015

comment:142

This has been fixed as part of #17634.

Nathann

@jdemeyer
Copy link

comment:143

This ticket wrote most docstrings as

__doc__ = r"""
docstring
"""

instead of the usual

r"""
docstring
"""

Does anybody remember why it was done this way? See #20633.

@nthiery
Copy link
Contributor Author

nthiery commented May 20, 2016

comment:144

Replying to @jdemeyer:

This ticket wrote most docstrings as

__doc__ = r"""
docstring
"""

instead of the usual

r"""
docstring
"""

Does anybody remember why it was done this way? See #20633.

I just answered there. In short, I can't recall a good reason. Thanks for catching this.

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

No branches or pull requests

7 participants