Opened 6 years ago

Closed 5 years ago

Last modified 3 years ago

#16256 closed enhancement (fixed)

Reorganize the documentation indexes into src/sage/combinat

Reported by: nthiery Owned by:
Priority: major Milestone: sage-6.4
Component: documentation Keywords: combinatorics, thematic index, quickref
Cc: sage-combinat, ncohen, tscrim, aschilling, vdelecroix Merged in:
Authors: Nicolas M. Thiéry, Jean-Pierre Flori Reviewers: Anne Schilling, Nathann Cohen
Report Upstream: N/A Work issues:
Branch: 823c116 (Commits) Commit:
Dependencies: #16058, #17189 Stopgaps:

Description (last modified by nthiery)

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 #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 #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.

Change History (144)

comment:1 Changed 6 years ago by nthiery

  • Description modified (diff)

comment:2 Changed 6 years ago by ncohen

  • Branch set to u/nthiery/16058-combinat-doc-index
  • Commit set to 391f7ff599040fc4f3e306d95e9941a9e0127a3b

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 <module>.__init__.py to <module>
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

comment:3 Changed 6 years ago by ncohen

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

comment:4 follow-ups: Changed 6 years ago by nthiery

  • Branch u/nthiery/16058-combinat-doc-index deleted
  • Commit 391f7ff599040fc4f3e306d95e9941a9e0127a3b deleted

Replying to ncohen:

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.

2) 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 :-)

3) Where are the combinatorial designs ?

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

4) 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.

comment:5 Changed 6 years ago by nthiery

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!

comment:6 Changed 6 years ago by ncohen

?....

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

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

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

comment:8 in reply to: ↑ 4 Changed 6 years ago by nthiery

Replying to nthiery:

I'll push / update the web page shortly.

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

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

Replying to ncohen:

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.

comment:10 Changed 6 years ago by nthiery

  • Branch set to u/nthiery/reorganize_the_documentation_indexes_into_src_sage_combinat

comment:11 Changed 6 years ago by nthiery

  • Commit set to d3284073bb560acb669287ef1c7208e8b384cbdd

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 <module>.__init__.py to <module>
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

comment:12 Changed 6 years ago by nthiery

  • Status changed from new to needs_info
  • Work issues set to gather comments by showing it around, and work on the listed TODO's

comment:13 Changed 6 years ago by nthiery

  • Description modified (diff)

comment:14 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:15 Changed 5 years ago by aschilling

  • Cc aschilling added

comment:16 follow-ups: Changed 5 years ago by jpflori

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.

comment:17 in reply to: ↑ 16 ; follow-up: Changed 5 years ago by ncohen

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

comment:18 follow-up: Changed 5 years ago by jpflori

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

comment:19 in reply to: ↑ 18 Changed 5 years ago by ncohen

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

comment:20 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:21 Changed 5 years ago by git

  • Commit changed from d3284073bb560acb669287ef1c7208e8b384cbdd to b3b708b629752a5eee482b62ba89aa0f918c2cc3

Branch pushed to git repo; I updated commit sha1. New commits:

b3b708bMerge branch 't/16256/reorganize_the_documentation_indexes_into_src_sage_combinat' into combinat/reorganize_the_documentation_indexes_into_src_sage_combinat-16256

comment:22 in reply to: ↑ 16 Changed 5 years ago by nthiery

  • Description modified (diff)

Hi Jean-Pierre,

Catching up with sage-trac e-mails ... sorry I had missed this one.

Replying to jpflori:

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

This ticket just reorganizes the documentation so that things get grouped nicely by sage module. But it should make it trivial to split the documentation on top of it in #14582.

Where you could help:

  • Giving your point of view on the approach taken here.
  • If you have already played with splitting sphinx documents, giving me a hand for #14582
  • And of course proofreading if you feel like it :-)

Cheers,

Nicolas

comment:23 Changed 5 years ago by git

  • Commit changed from b3b708b629752a5eee482b62ba89aa0f918c2cc3 to 9d388dbb8cee88a345e5d007d9284e0266b8b48b

Branch pushed to git repo; I updated commit sha1. New commits:

9d388dbMerge branch 'develop=6.4 beta2' into combinat/reorganize_the_documentation_indexes_into_src_sage_combinat-16256

comment:24 Changed 5 years ago by git

  • Commit changed from 9d388dbb8cee88a345e5d007d9284e0266b8b48b to a23c12fafb58d03e371d47102bc490a7142ab7ab

Branch pushed to git repo; I updated commit sha1. New commits:

af1680b16256: Merge branch 'develop=6.4 beta2' into combinat/reorganize_the_documentation_indexes_into_src_sage_combinat-16256 (cont)
a23c12f16256: updated sage/combinat/module_list.rst

comment:25 follow-up: Changed 5 years ago by ncohen

  • Cc vdelecroix added

Yo !

The link toward the result is not availble anymore, and I don't like what you are doing with combinat/designs.rst (i.e. removing the file). How will this page be replaced ?

Nathann

comment:26 follow-up: Changed 5 years ago by ncohen

Oh. By the equivalent __init__.py. Nevermind :-P

Nathann

comment:27 in reply to: ↑ 17 ; follow-up: Changed 5 years ago by nthiery

Replying to ncohen:

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 ...

Indeed, as a side effect, it adds a few files that were missing from the reference manual; but this should not make a real difference.

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

Be my guest and show the example by doing your share :-)

The produced documentation for the sections about graphs and designs with this branched applied can be found respectively in:

The sources are in the corresponding __init__.py files:

(or just clone this branch as usual)

What's to be done about those sections:

  • Checking that it looks good from the user point of view
  • Choosing better entry points when appropriate (e.g. sometime it's more natural to point directly to a class than to the module defining it).
  • Checking that it looks natural from the dev point of view
  • Checking that nothing got lost in the operation
  • Proofreading for typos

Thanks in advance,

Nicolas

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

comment:28 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:29 in reply to: ↑ 25 Changed 5 years ago by nthiery

Replying to ncohen:

The link toward the result is not availble anymore

Race-condition: I was precisely rebuilding it :-) The new link should work.

comment:30 in reply to: ↑ 26 Changed 5 years ago by nthiery

Replying to ncohen:

Oh. By the equivalent __init__.py. Nevermind :-P

Another race condition :-)

comment:31 in reply to: ↑ 27 ; follow-up: Changed 5 years ago by ncohen

Yooooooooo !

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

Be my guest and show the example by doing your share :-)

Ahahah. Big political tickets are not my style, sorry. I change the things I know, bit by bit :-P

The produced documentation for the sections about graphs and designs with this branched applied can be found respectively in:

Thanks ! Well, it looks cool, I mean... The design page looks exactly like it looks without your patch applied, so it's all good to me.

I am of course against having a "Graph" entry in combinat's doc. It just does not belong there, and it is not because combinat users may need other features of Sage that you should copy/paste their doc in your own document. If you can't be convinced otherwise, however, it would be better to redirect them toward the index of the graph help, i.e. this page :

http://www.sagemath.org/doc/reference/graphs/index.html

They would get what they asked for: the part of the doc dedicated to graphs.

  • Checking that it looks good from the user point of view

The main page looks okay. The entries in the main combinat page may be sorted (Algebraic Combinatorics appears toward the end of the list)

Given that you do not have so many entries in "Thematic indexes", you could merge it with Dynamical Systems and Miscellaneous too.

It would avoid having a Miscellaneous entry in the Miscellaneous section too.

On the other hand, as soon as you click on the entries of the main page things begin to get messy. You can reach Words by its entry on the main page, and through "Enumerated sets and combinatorial objects"

Many entries only display the name of a class rather than a full-text description, as the all files you add do not necessarily have a module doc. Sooo well. Maybe you should first add at least one doc line to the top of each file in combinat before anything else.

  • Choosing better entry points when appropriate (e.g. sometime it's more natural to point directly to a class than to the module defining it).
  • Checking that it looks natural from the dev point of view

It's better this way. The doc is closer to the code, that's nice.

Nathann

comment:32 Changed 5 years ago by git

  • Commit changed from a23c12fafb58d03e371d47102bc490a7142ab7ab to eb78962f4978bd50629919ad3a88ee1549f4c380

Branch pushed to git repo; I updated commit sha1. New commits:

eb7896216256: reference graph theory, coding, and dynamics as 'related topics'; minor improvements

comment:33 Changed 5 years ago by git

  • Commit changed from eb78962f4978bd50629919ad3a88ee1549f4c380 to 4ea19e8e8c4b00e9c64247b49fa4ce55c55647b8

Branch pushed to git repo; I updated commit sha1. New commits:

b872e5716256: use ~ when referencing classes and functions
4ea19e816256: added title to sage.combinat.perm_grps.permutation_groups_catalog

comment:34 in reply to: ↑ 31 ; follow-ups: Changed 5 years ago by nthiery

Hi Nathann,

Thanks for looking this up and your comments!

Replying to ncohen:

Thanks ! Well, it looks cool, I mean... The design page looks exactly like it looks without your patch applied, so it's all good to me.

:-)

I am of course against having a "Graph" entry in combinat's doc. It just does not belong there, and it is not because combinat users may need other features of Sage that you should copy/paste their doc in your own document.

It's meant as a cross-reference, not as "include the doc here". I clarified this by putting the link in a SEEALSO section.

The point of this cross ref is that a user interested in combinatorics has some good chances to be interested in graph theory too. In general, being able to easily add such crossrefs is actually one of my big motivation for this ticket, as the lack of such crossrefs is one of the weakest points of the reference manual.

it would be better to redirect them toward the index of the graph help, i.e. this page :

http://www.sagemath.org/doc/reference/graphs/index.html

They would get what they asked for: the part of the doc dedicated to graphs.

Good point. To achieve this, I allowed myself to add a label "sage.graphs" at the top of doc/en/reference/graphs/index.rst.

The main page looks okay. The entries in the main combinat page may be sorted (Algebraic Combinatorics appears toward the end of the list)

Good point as well. I had tried to sort them semantically, but it's actually so close from alphabetic order that it might as well be alphabetic.

Given that you do not have so many entries in "Thematic indexes", you could merge it with Dynamical Systems and Miscellaneous too.

Right. I had not noticed there was now a sage.dynamics folder. So I cross-ref'd it instead, and added a link back from there to e_one_star.

Vincent: tell me if that sounds good to you. Refactoring the index for sage.dynamics as is done here for sage.combinat will probably allow for a nicer looking crossref back; but I leave that to a later ticket.

It would avoid having a Miscellaneous entry in the Miscellaneous section too.

It's pretty ridiculous indeed. Not sure how to do this since that's the title of the module, and given the content, I did not find a much more compelling title.. Well, I renamed the section into "Utilities" :-)

On the other hand, as soon as you click on the entries of the main page things begin to get messy. You can reach Words by its entry on the main page, and through "Enumerated sets and combinatorial objects"

Yes, and this is on purpose: the organization of code according to a tree layout is always somewhat arbitrary, whereas users looking for a given feature may have different ideas in mind (you can come to words by thinking "what objects can I enumerate through", or "word theory", or "automata", or ...); so it's good that the given feature be referenced from several places/indexes. Again I believe that's a selling point for this ticket.

Many entries only display the name of a class rather than a full-text description, as the all files you add do not necessarily have a module doc. Sooo well. Maybe you should first add at least one doc line to the top of each file in combinat before anything else.

Yes, there is a technical sphinx point to investigate here.

In many cases, it's better to point directly to a class (or function) instead of a module. This can be because the rest of the module consists of not-so-interesting utility functions; or because a module contains several classes that deserve to be highlighted; or because the nice introductory documentation is written in the class.

Now, the question is: when referencing a class (or function), how to display the one-line description of that class rather than its name. Of course, one can always duplicate by hand that description, but that's ugly.

As a first step, I have added a ~ so that at least we only see the name of the class and not its module prefix. That does not look great in the output in term of consistency of style, but I believe that's ok for now and should not delay this ticket in case configuring Sphinx turns out to take a bit of time.

It's better this way. The doc is closer to the code, that's nice.

Cool.

Cheers,

Nicolas


New commits:

b872e5716256: use ~ when referencing classes and functions
4ea19e816256: added title to sage.combinat.perm_grps.permutation_groups_catalog

New commits:

b872e5716256: use ~ when referencing classes and functions
4ea19e816256: added title to sage.combinat.perm_grps.permutation_groups_catalog

comment:35 in reply to: ↑ 34 ; follow-up: Changed 5 years ago by aschilling

I think it would be a good idea to include crystals, root systems, symmetric functions etc in the thematic indexes. Also, what is the problems linking the thematic tutorials? The user should be able to access them from the front page.

Anne

comment:36 in reply to: ↑ 34 Changed 5 years ago by ncohen

Yoooooooooo !! (from my office at the LRI)

I am of course against having a "Graph" entry in combinat's doc. It just does not belong there, and it is not because combinat users may need other features of Sage that you should copy/paste their doc in your own document.

It's meant as a cross-reference, not as "include the doc here". I clarified this by putting the link in a SEEALSO section.

The point of this cross ref is that a user interested in combinatorics has some good chances to be interested in graph theory too. In general, being able to easily add such crossrefs is actually one of my big motivation for this ticket, as the lack of such crossrefs is one of the weakest points of the reference manual.

I wanted to give it a look but the doc does not compile with your branch. Several deprecation warnings appear at first, then there is this

OSError: [dynamics ] /home/ncohen/.Sage/src/doc/en/reference/dynamics/index.rst:14: WARNING: undefined label: sage.combinat.e_one_star (if the link has no caption the label must precede a section header)

I don't have anything against your fetishism for cross-references in general, though in this case you cannot exactly say that the doc for graphs is hard to find in Sage. It has its own entry in the first page of the reference manual :-P

This being said, you will find in the "graph" page a link toward the "IncidenceStructures?" defined in combinat/designs, just in case a guy who needs a hypergraph class ends up there. But this is way harder to find than Graphs.

Many entries only display the name of a class rather than a full-text description, as the all files you add do not necessarily have a module doc. Sooo well. Maybe you should first add at least one doc line to the top of each file in combinat before anything else.

Yes, there is a technical sphinx point to investigate here.

In many cases, it's better to point directly to a class (or function) instead of a module. This can be because the rest of the module consists of not-so-interesting utility functions; or because a module contains several classes that deserve to be highlighted; or because the nice introductory documentation is written in the class.

Well... I solved this the other way, by adding interesting doc in the module itself. While it's not very natural to look at the doc of a module when you use Sage, these pages are the html page that appear on google when you look for help. Sooooooo I guess that quite a lot of person end up reading them.

When the module consists of many "not very useful" functions and a few interesting things, you can chose to only mention the important function in the module's head, or to emphasize those that are actually useful.

http://www.sagemath.org/doc/reference/combinat/sage/combinat/designs/latin_squares.html http://www.sagemath.org/doc/reference/combinat/sage/combinat/designs/incidence_structures.html

A couple of words, an index of functions, and you can point to the module directly.

See you later if you come to the lab !

Nathann

comment:37 follow-up: Changed 5 years ago by ncohen

Oh. It seems that if you "try harder" the doc can be built:

make doc-clean && make # will not work

But

make doc-clean; make;make;make;make;make # should eventually work

When you build a module which refers to a module that has not been built the reference fails, but if you type "make" again the failed document will not be rebuilt, so the problem is skipped. With a bit of luck you will build the other document someday, and THEN you will be able to build the first.

Anyway, this may be a problem, at least for Volker.

Nathann

comment:38 in reply to: ↑ 37 Changed 5 years ago by nthiery

Replying to ncohen:

Oh. It seems that if you "try harder" the doc can be built:

make doc-clean && make # will not work

But

make doc-clean; make;make;make;make;make # should eventually work

Yes, sorry, I got also bitten by this and was actually about to put a warning about this on the ticket. We investigated this a bit this morning with Florent, and it's likely to be just a glitch in the warning reporting while in the first pass of the compilation. This definitely has to be fixed before this ticket can go in.

I'll come to the lab tomorrow. Today I am handling teaching organization in the vallée ...

Cheers,

Nicolas

comment:39 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:40 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:41 in reply to: ↑ 35 Changed 5 years ago by nthiery

Hi Anne!

Thanks for looking!

Replying to aschilling:

I think it would be a good idea to include crystals, root systems, symmetric functions etc in the thematic indexes.

Ok, will do then!

Also, what is the problems linking the thematic tutorials? The user should be able to access them from the front page.

Just a sphinx issue for how to generate properly the link (the reference manual is built before the thematic tutorials, so can't access its intersphinx database of labels). I'll investigate this with Florent, but we can always workaround this for now with a hardcoded html link.

Cheers,

Nicolas

comment:42 Changed 5 years ago by git

  • Commit changed from 4ea19e8e8c4b00e9c64247b49fa4ce55c55647b8 to 29734cfb075a969fdc8885bf1c9d4797531cc2fd

Branch pushed to git repo; I updated commit sha1. New commits:

7b6637516256: temporarily hard coded html links to the thematic tutorials
29734cf16256: added direct links to the indexes on crystals, quiver algebras, root systems, ...

comment:43 follow-up: Changed 5 years ago by git

  • Commit changed from 29734cfb075a969fdc8885bf1c9d4797531cc2fd to 3f0ef5ab3c0bda2e5ee3730fd48f1186e56f0401

Branch pushed to git repo; I updated commit sha1. New commits:

3f0ef5a16256: ReST typo fix on previous commit

comment:44 in reply to: ↑ 43 ; follow-up: Changed 5 years ago by aschilling

Hi!

Here are some brief comments:

  • The title "Bases for ." in "Symmetric Functions in Non-Commuting Variables" looks weird.
  • I am not sure why all these catalogs are
        Catalog Of Crystals
        Catalog Of Elementary Crystals
        Catalog Of Crystal Models For
        Catalog Of Crystal Models For Kirillov-Reshetikhin Crystals
    

are listed separately. The third title seems to be missing something. In principle the last three are subsumed under the link "Catalog Of Crystals".

  • The link under "Introductory material" for crystals "Crystals – This overview" is broken.
  • In general, math displays seem to be missing from titles which make them look strange and incomplete.
  • The alphabetical module list does not seem to be alphabetical.

Best,

Anne

comment:45 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:46 Changed 5 years ago by git

  • Commit changed from 3f0ef5ab3c0bda2e5ee3730fd48f1186e56f0401 to 7af0a5d5d319ad00ba2700a7037c1e2664820aaa

Branch pushed to git repo; I updated commit sha1. New commits:

8602a2d16256: pass of proofreading on all combinat documentation indexes, handling a bunch of TODO's there.
7af0a5d16256: improved sage.combinat.quickref

comment:47 Changed 5 years ago by git

  • Commit changed from 7af0a5d5d319ad00ba2700a7037c1e2664820aaa to 2252c4453b5788a3c44fe05100fc0a9f93ea6919

Branch pushed to git repo; I updated commit sha1. New commits:

2252c4416256: improved sage.combinat.crystals's index

comment:48 in reply to: ↑ 44 ; follow-up: Changed 5 years ago by nthiery

Replying to aschilling:

Here are some brief comments:

Thanks!

All tests pass now :-)

I would suggest to also add Tableaux (with usual and some affine versions), Cores (perhaps close to Partitions) and Permutations (perhaps close to WeylGroups? and RootSystems?).

Good point: I have added examples with permutations, tableaux, dyckwords, trees. I left out affine tableaux and cores for now, as they seem sound to me a bit exotic for a general combinatorics quickref. They definitely would make sense in a more specialized quickref though.

What do you think?

  • I am not sure why all these catalogs are
        Catalog Of Crystals
        Catalog Of Elementary Crystals
        Catalog Of Crystal Models For
        Catalog Of Crystal Models For Kirillov-Reshetikhin Crystals
    

are listed separately. The third title seems to be missing something. In principle the last three are subsumed under the link "Catalog Of Crystals".

Good point. I removed them.

By the way: what about removing the definition of crystals from sage.combinat.crystals.catalog? It duplicates that of sage.combinat.crystals.crystals, and the user interested just in the catalog needs to scroll down.

  • The title "Bases for ." in "Symmetric Functions in Non-Commuting Variables" looks weird.
  • In general, math displays seem to be missing from titles which make them look strange and incomplete.

Indeed: sphinx does not handle math in (references to) titles. Since this was already broken beforehand, I was thinking of not fixing this in this ticket to avoid being too invasive. Now, if you give me a green light for crystals, I can fix those. Idem for the above change to the catalog.

  • The link under "Introductory material" for crystals "Crystals – This overview" is broken.

Fixed. More precisely work around (the usual issue with linking to the thematic tutorials).

  • The alphabetical module list does not seem to be alphabetical.

It's alphabetical in term of module names, but not module titles. Not sure what's the best approach here; possibly including the name of the module.

Cheers,

Nicolas


New commits:

8602a2d16256: pass of proofreading on all combinat documentation indexes, handling a bunch of TODO's there.
7af0a5d16256: improved sage.combinat.quickref
2252c4416256: improved sage.combinat.crystals's index

comment:49 Changed 5 years ago by nthiery

  • Status changed from needs_info to needs_review
  • Work issues changed from gather comments by showing it around, and work on the listed TODO's to Fix sphinx issues.

I have taken care of a bunch of the TODO's. The other could possibly be taken care later on since they are anyway further enhancements w.r.t. the previous state. The big remaining thing are the Sphinx technicalities.

I am putting this to needs review to get the buildbot to munch on it.

comment:50 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:51 in reply to: ↑ 48 ; follow-up: Changed 5 years ago by aschilling

All tests pass now :-)

That is surprising given some of the syntax

sage: CoxeterGroup(["B",3]).some_flashy_feature()

or

sage: M = MultivariatePolynomials('x0,x1,x2,x3')
sage: M(...).action??? S.

Good point: I have added examples with permutations, tableaux, dyckwords, trees. I left out affine tableaux and cores for now, as they seem sound to me a bit exotic for a general combinatorics quickref. They definitely would make sense in a more specialized quickref though.

What do you think?

That looks good!

  • I am not sure why all these catalogs are
        Catalog Of Crystals
        Catalog Of Elementary Crystals
        Catalog Of Crystal Models For
        Catalog Of Crystal Models For Kirillov-Reshetikhin Crystals
    

are listed separately. The third title seems to be missing something. In principle the last three are subsumed under the link "Catalog Of Crystals".

Good point. I removed them.

Perhaps I am looking at an old version of the compiled documentation, but on http://sage.math.washington.edu/home/nthiery/sage/src/doc/output/html/en/reference/combinat/sage/combinat/crystals/__init__.html#sage-combinat-crystals they are still there.

By the way: what about removing the definition of crystals from sage.combinat.crystals.catalog? It duplicates that of sage.combinat.crystals.crystals, and the user interested just in the catalog needs to scroll down.

Unlike stated on the catalogue site, the documentation does not completely duplicate the documentation in sage.combinat.crystals.crystals. I suggest moving it to sage.combinat.crystals.crystals.

This link http://sage.math.washington.edu/home/nthiery/sage/src/doc/output/html/en/reference/combinat/sage/combinat/crystals/__init__.html#sage-combinat-crystals on the crystal quickref site seems to be broken.

The link to sage.combinat.posets on http://sage.math.washington.edu/home/nthiery/sage/src/doc/output/html/en/reference/combinat/sage/combinat/quickref.html seems to be broken.

Indeed: sphinx does not handle math in (references to) titles. Since this was already broken beforehand, I was thinking of not fixing this in this ticket to avoid being too invasive. Now, if you give me a green light for crystals, I can fix those. Idem for the above change to the catalog.

Sure.

  • The link under "Introductory material" for crystals "Crystals – This overview" is broken.

Fixed. More precisely work around (the usual issue with linking to the thematic tutorials).

For me that link is still broken. I am confused (since you say it is supposed to be the link to the thematic tutorial): That link appear two lines below that under "The Lie Methods and Related Combinatorics thematic tutorial" Why have two links to the thematic tutorial?

  • The alphabetical module list does not seem to be alphabetical.

It's alphabetical in term of module names, but not module titles. Not sure what's the best approach here; possibly including the name of the module.

That is very confusing to the user and developer since it says explicitly "alphabetical". From just looking at the list, it is not clear that the modules are listed alphabetical. Please include the name of the module or not flatten the list.

Best,

Anne

comment:52 in reply to: ↑ 51 Changed 5 years ago by nthiery

Replying to aschilling:

That is surprising given some of the syntax ... Perhaps I am looking at an old version of the compiled documentation, but on http://sage.math.washington.edu/home/nthiery/sage/src/doc/output/html/en/reference/combinat/sage/combinat/crystals/__init__.html#sage-combinat-crystals they are still there. ... various broken links ...

Sorry, I screwed up the rsync. Ok, now it's scripted, so that I don't screw up again next time.

Unlike stated on the catalogue site, the documentation does not completely duplicate the documentation in sage.combinat.crystals.crystals. I suggest moving it to sage.combinat.crystals.crystals.

Sure.

Ok, will do! When I find the time ...

For me that link is still broken. I am confused (since you say it is supposed to be the link to the thematic tutorial): That link appear two lines below that under "The Lie Methods and Related Combinatorics thematic tutorial" Why have two links to the thematic tutorial?

The first points to the introduction in sage.combinat.crystals.crystals, the second one to the thematic tutorial.

That is very confusing to the user and developer since it says explicitly "alphabetical". From just looking at the list, it is not clear that the modules are listed alphabetical. Please include the name of the module or not flatten the list.

Hmm, more sphinx to do ...

Cheers,

Nicolas

comment:53 Changed 5 years ago by git

  • Commit changed from 2252c4453b5788a3c44fe05100fc0a9f93ea6919 to d4b6d28935f08d2bbc4215ee7fb36bb0b1541369

Branch pushed to git repo; I updated commit sha1. New commits:

d4b6d2816256: merged the two introduction of crystals (in the catalog.py an crystals.py) and further little improvements

comment:54 Changed 5 years ago by git

  • Commit changed from d4b6d28935f08d2bbc4215ee7fb36bb0b1541369 to 2872210a0f22f940a6b808514eb6ed0b27f1f9a7

Branch pushed to git repo; I updated commit sha1. New commits:

287221016256: proofreading previous commit

comment:55 follow-up: Changed 5 years ago by jpflori

Anne, are you happy with the latest version (except it seems it needs rebasing)? It would be very nice to get this merged!

comment:56 Changed 5 years ago by ncohen

  • Status changed from needs_review to needs_work

It would be cool to have an updated branch first to be able to see all accumulated changes on the trac ticket.

Nathann

comment:57 in reply to: ↑ 55 Changed 5 years ago by aschilling

Replying to jpflori:

Anne, are you happy with the latest version (except it seems it needs rebasing)? It would be very nice to get this merged!

From the comments above it seems like Nicolas has not yet made all the changes. It is not possible to look at the branch right now, so I cannot comment more.

Anne

comment:58 Changed 5 years ago by jpflori

  • Branch changed from u/nthiery/reorganize_the_documentation_indexes_into_src_sage_combinat to u/jpflori/16256
  • Commit 2872210a0f22f940a6b808514eb6ed0b27f1f9a7 deleted

Have fun.

comment:59 Changed 5 years ago by jpflori

  • Branch changed from u/jpflori/16256 to u/jpflori/ticket/16256
  • Commit set to 28969d79b6a43abf1668ae6a0aef6a52ba0777c3

Last 10 new commits:

4ea19e816256: added title to sage.combinat.perm_grps.permutation_groups_catalog
7b6637516256: temporarily hard coded html links to the thematic tutorials
29734cf16256: added direct links to the indexes on crystals, quiver algebras, root systems, ...
3f0ef5a16256: ReST typo fix on previous commit
8602a2d16256: pass of proofreading on all combinat documentation indexes, handling a bunch of TODO's there.
7af0a5d16256: improved sage.combinat.quickref
2252c4416256: improved sage.combinat.crystals's index
d4b6d2816256: merged the two introduction of crystals (in the catalog.py an crystals.py) and further little improvements
287221016256: proofreading previous commit
28969d7Merge remote-tracking branch 'trac/develop' into ticket/16256

comment:60 Changed 5 years ago by ncohen

This is incorrect, at least for combinatorial designs.

Nathann

comment:61 Changed 5 years ago by jpflori

This is not very helpful. What is incorrect?

comment:62 Changed 5 years ago by ncohen

What the ticket removes from the current file is not what is being added into the new one.

comment:63 follow-up: Changed 5 years ago by tscrim

My 2 cents, I liked having the specific Cartan type data in a separate file to not clutter or distract.

Also a catalog for algebras is #16219.

comment:64 Changed 5 years ago by nthiery

Thanks for your interest, feedback, and work. At this point, the blocker is the Sphinx compilation issue with crosslinks, and I am stuck there. Once this is taken care of (Florent, please!!!), finalizing the rebase upon the latest beta will be a breeze.

Cheers,

Nicolas

comment:65 Changed 5 years ago by nthiery

  • Dependencies changed from #16058 to #16058, #17189

comment:66 Changed 5 years ago by nthiery

I worked with Florent this afternoon on a fix for #17189 which I created for the Sphinx compilation issue.

comment:67 Changed 5 years ago by nthiery

  • Branch changed from u/jpflori/ticket/16256 to u/nthiery/ticket/16256

comment:68 in reply to: ↑ 63 Changed 5 years ago by nthiery

  • Commit changed from 28969d79b6a43abf1668ae6a0aef6a52ba0777c3 to cef0ee08f43e178766f6a737b2d4ea18ac8fd90c

Replying to tscrim:

My 2 cents, I liked having the specific Cartan type data in a separate file to not clutter or

distract.

Good question. It's at the end, and it gives at a glance an overview of how things are organized. Also, I would not know where to split (e.g. where to put the semi generic stuff like type_affine, type_dual). So at this point I vote for keeping things as they are. And if the file grows further, or many more types are added (e.g. for complex reflection groups), then we will split that off.

Also a catalog for algebras is #16219.

Thanks for the pointer and for that catalog! I reviewed it, and will merge it in as a dependency as soon as it's finalized.


New commits:

b7e0a6917189: upon the first pass of the documentation compilation, warnings don't trigger an exception
fb43b1917189: referee's suggestions: only undefined warnings don't trigger an exception
eaff50b16256: Merge branch 't/17189/upon_the_first_pass_of_the_documentation_compilation__undefined_label_warnings_should_not_trigger_an_exception'
cef0ee016256: fixed missing links from the previous merge

comment:69 Changed 5 years ago by nthiery

  • Status changed from needs_work to needs_review
  • Work issues Fix sphinx issues. deleted

comment:70 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:71 follow-ups: Changed 5 years ago by nthiery

For the record: the doc compiles, and all long tests pass but for one easy failure:

Failed example:
    sage.misc.dev_tools.load_submodules(sage.combinat)
Expected:
    load sage.combinat.cluster_algebra_quiver.cluster_seed... suceeded
    load sage.combinat.cluster_algebra_quiver.mutation_class... suceeded
    ...
    load sage.combinat.words.suffix_trees... suceeded

Remains to double check the doc building with -warn-links.

comment:72 in reply to: ↑ 71 ; follow-up: Changed 5 years ago by aschilling

Some quick comments that I think should really be fixed:

  • The alphabetical module list is not alphabetical
  • Some links to thematic tutorials are broken (perhaps you did not put them on the website?).

Anne

comment:73 follow-up: Changed 5 years ago by kcrisman

On a related note, would third-party links still work with this reorganization? (Similarly to something we had to do with the change in the Sage reference manual to build in parallel.) My apologies if this is addressed somewhere above, I didn't find it on a quick glance.

comment:74 in reply to: ↑ 73 Changed 5 years ago by nthiery

Replying to kcrisman:

On a related note, would third-party links still work with this reorganization? (Similarly to something we had to do with the change in the Sage reference manual to build in parallel.) My apologies if this is addressed somewhere above, I didn't find it on a quick glance.

You mean cross links between different documents of the reference manual? Yes indeed; actually the reorganization encourages such links.

As for other links, it should not make a difference.

comment:75 in reply to: ↑ 72 ; follow-ups: Changed 5 years ago by jpflori

Replying to aschilling:

Some quick comments that I think should really be fixed:

  • The alphabetical module list is not alphabetical

That's how ls sorts stuff, I don't think that's a terrible choice. Is it the way capitals and underscores are dealt with that you don't like? This can be easily changed.

  • Some links to thematic tutorials are broken (perhaps you did not put them on the website?).

Which ones?

comment:76 in reply to: ↑ 75 ; follow-ups: Changed 5 years ago by jpflori

Replying to jpflori:

Replying to aschilling:

Some quick comments that I think should really be fixed:

  • The alphabetical module list is not alphabetical

That's how ls sorts stuff, I don't think that's a terrible choice. Is it the way capitals and underscores are dealt with that you don't like? This can be easily changed.

Oh, I see, what is in the doc is not in alphabetical order indeed... Only what is in the file used to generate the doc.

  • Some links to thematic tutorials are broken (perhaps you did not put them on the website?).

Which ones?

comment:77 in reply to: ↑ 76 Changed 5 years ago by jpflori

Replying to jpflori:

Replying to jpflori:

Replying to aschilling:

Some quick comments that I think should really be fixed:

  • The alphabetical module list is not alphabetical

That's how ls sorts stuff, I don't think that's a terrible choice. Is it the way capitals and underscores are dealt with that you don't like? This can be easily changed.

Oh, I see, what is in the doc is not in alphabetical order indeed... Only what is in the file used to generate the doc.

  • Some links to thematic tutorials are broken (perhaps you did not put them on the website?).

Which ones?

Were you thinking about these ones:

comment:78 in reply to: ↑ 71 Changed 5 years ago by jpflori

Replying to nthiery:

For the record: the doc compiles, and all long tests pass but for one easy failure:

Failed example:
    sage.misc.dev_tools.load_submodules(sage.combinat)
Expected:
    load sage.combinat.cluster_algebra_quiver.cluster_seed... suceeded
    load sage.combinat.cluster_algebra_quiver.mutation_class... suceeded
    ...
    load sage.combinat.words.suffix_trees... suceeded

Remains to double check the doc building with -warn-links.

Is there any easy way to do this for the combinat folder only? And I guess --warn-links must only be passed in the second pass?

comment:79 in reply to: ↑ 76 Changed 5 years ago by jpflori

Replying to jpflori:

Replying to jpflori:

Replying to aschilling:

Some quick comments that I think should really be fixed:

  • The alphabetical module list is not alphabetical

That's how ls sorts stuff, I don't think that's a terrible choice. Is it the way capitals and underscores are dealt with that you don't like? This can be easily changed.

Oh, I see, what is in the doc is not in alphabetical order indeed... Only what is in the file used to generate the doc.

So this will be more involved, I guess doing it properly would involved asking shpinx what it intends to use for each file and sort afterward.

  • Some links to thematic tutorials are broken (perhaps you did not put them on the website?).

Which ones?

comment:80 in reply to: ↑ 75 ; follow-up: Changed 5 years ago by aschilling

Replying to jpflori:

Replying to aschilling:

Some quick comments that I think should really be fixed:

  • The alphabetical module list is not alphabetical

That's how ls sorts stuff, I don't think that's a terrible choice. Is it the way capitals and underscores are dealt with that you don't like? This can be easily changed.

I found it confusing that it says on the top that the list is in alphabetical order, but then it is not. Since a user won't know what kind of sort is used, it is hard to guess (unless it is explained how the sort is done).

  • Some links to thematic tutorials are broken (perhaps you did not put them on the website?).

Which ones?

For example the link to the "Lie Tutorial" on http://sage.math.washington.edu/home/nthiery/sage/src/doc/output/html/en/reference/combinat/sage/combinat/crystals/__init__.html#sage-combinat-crystals

comment:81 in reply to: ↑ 80 ; follow-up: Changed 5 years ago by jpflori

Replying to aschilling:

Replying to jpflori:

Replying to aschilling:

Some quick comments that I think should really be fixed:

  • The alphabetical module list is not alphabetical

That's how ls sorts stuff, I don't think that's a terrible choice. Is it the way capitals and underscores are dealt with that you don't like? This can be easily changed.

I found it confusing that it says on the top that the list is in alphabetical order, but then it is not. Since a user won't know what kind of sort is used, it is hard to guess (unless it is explained how the sort is done).

Yes, I actually first checked the source file whence my misunderstanding. I propose to add more info on why this does not look alphabetical first and use sphinx to get a better ordering in a follow up ticket if you think it is ok.

  • Some links to thematic tutorials are broken (perhaps you did not put them on the website?).

Which ones?

For example the link to the "Lie Tutorial" on http://sage.math.washington.edu/home/nthiery/sage/src/doc/output/html/en/reference/combinat/sage/combinat/crystals/__init__.html#sage-combinat-crystals

Thanks! There is indeed a problem in that link, a spurious reference toward the end of the url. I'll fix it here.

comment:82 in reply to: ↑ 81 ; follow-up: Changed 5 years ago by aschilling

Replying to jpflori:

Replying to aschilling:

Replying to jpflori:

Replying to aschilling:

Some quick comments that I think should really be fixed:

  • The alphabetical module list is not alphabetical

That's how ls sorts stuff, I don't think that's a terrible choice. Is it the way capitals and underscores are dealt with that you don't like? This can be easily changed.

I found it confusing that it says on the top that the list is in alphabetical order, but then it is not. Since a user won't know what kind of sort is used, it is hard to guess (unless it is explained how the sort is done).

Yes, I actually first checked the source file whence my misunderstanding. I propose to add more info on why this does not look alphabetical first and use sphinx to get a better ordering in a follow up ticket if you think it is ok.

Or we could just remove the header claiming that this is an alphabetical ordering! I just think that that part is confusing for a user.

comment:83 in reply to: ↑ 82 Changed 5 years ago by jpflori

Replying to aschilling:

Or we could just remove the header claiming that this is an alphabetical ordering! I just think that that part is confusing for a user.

Very good idea!

I'll do that, add an explanation of the current sort, and a todo to make it better and add sublevels as well.

comment:84 follow-up: Changed 5 years ago by jpflori

  • Authors changed from Nicolas M. Thiéry to Nicolas M. Thiéry, Jean-Pierre Flori
  • Branch changed from u/nthiery/ticket/16256 to u/jpflori/ticket/16256
  • Commit changed from cef0ee08f43e178766f6a737b2d4ea18ac8fd90c to 06d581f3566243970190e4898eaa3ab94e1a6e15

Ok, please have another look at the current state.

Everything is not perfect but I feel this is a very useful step towards a better situation.


New commits:

7a14585Merge remote-tracking branch 'trac/develop' into ticket/16256
cf463b7Fix doctest (and a typo) after new combinat doc merge.
aef96aaUse more usual ordering for source files in combinat module list.
d365921Do not pretend th module list in combinat doc is in alphabetical order.
06d581fFix links from combinat doc to thematic tutorials.

comment:85 in reply to: ↑ 84 ; follow-up: Changed 5 years ago by aschilling

Replying to jpflori:

Ok, please have another look at the current state.

I will try to look at it tonight. I have a lot of teaching ahead of me now!

comment:86 in reply to: ↑ 85 ; follow-ups: Changed 5 years ago by aschilling

Ok, my computer only just finished compiling ... here are some more comments:

  • The link to /sage/src/doc/output/html/thematic_tutorials/algebraic_combinatorics.html in sage/src/doc/output/html/en/reference/combinat/sage/combinat/algebraic_combinatorics.html does not seem to work for me. Actually none of the thematic tutorial links seem to work for me.
  • Some of the links on the page sage/src/doc/output/html/en/reference/combinat/sage/combinat/ncsf_qsym/init.html do not have titles.
  • On the page sage/src/doc/output/html/en/reference/combinat/sage/combinat/ncsym/init.html it says "Basis for <blank>"

So much for now!

Anne

comment:87 Changed 5 years ago by git

  • Commit changed from 06d581f3566243970190e4898eaa3ab94e1a6e15 to 7b753a2c64af971fc37d1fffa8f95e6fac1e0438

Branch pushed to git repo; I updated commit sha1. New commits:

7b753a2Revert "Fix links from combinat doc to thematic tutorials."

comment:88 Changed 5 years ago by jpflori

Ok, I checked the html output on my machine and what Nicolas posted on the web was just not up to date and no fix was needed.

comment:89 follow-up: Changed 5 years ago by jpflori

...as far as thematic tutos links were concerned.

comment:90 in reply to: ↑ 86 Changed 5 years ago by jpflori

Replying to aschilling:

Ok, my computer only just finished compiling ... here are some more comments:

  • On the page sage/src/doc/output/html/en/reference/combinat/sage/combinat/ncsym/init.html it says "Basis for <blank>"

This involves latex. What is strange is that it is ok in module_list.html.

comment:91 in reply to: ↑ 86 ; follow-up: Changed 5 years ago by jpflori

Replying to aschilling:

Ok, my computer only just finished compiling ... here are some more comments:

  • Some of the links on the page sage/src/doc/output/html/en/reference/combinat/sage/combinat/ncsf_qsym/init.html do not have titles.

There are some others all over the place. And some font discrepancies between the titles in some lists. I would be inclined to fix all of these later as I would not they they were introduced here.

comment:92 in reply to: ↑ 91 ; follow-up: Changed 5 years ago by jpflori

Replying to jpflori:

Replying to aschilling:

Ok, my computer only just finished compiling ... here are some more comments:

  • Some of the links on the page sage/src/doc/output/html/en/reference/combinat/sage/combinat/ncsf_qsym/init.html do not have titles.

There are some others all over the place. And some font discrepancies between the titles in some lists. I would be inclined to fix all of these later as I would not they they were introduced here.

Or not, it looks better here: http://www.sagemath.org/doc/reference/combinat/ncsf_qsym.html

comment:93 in reply to: ↑ 92 Changed 5 years ago by jpflori

Replying to jpflori:

Replying to jpflori:

Replying to aschilling:

Ok, my computer only just finished compiling ... here are some more comments:

  • Some of the links on the page sage/src/doc/output/html/en/reference/combinat/sage/combinat/ncsf_qsym/init.html do not have titles.

There are some others all over the place. And some font discrepancies between the titles in some lists. I would be inclined to fix all of these later as I would not they they were introduced here.

Or not, it looks better here: http://www.sagemath.org/doc/reference/combinat/ncsf_qsym.html

Got it: Nicolas replaced :ref: with reference to the module by :class: with reference to the main class which is a good thing. Adding a tilde in front of the class ref would make things a little more readable already.

comment:94 in reply to: ↑ 89 ; follow-up: Changed 5 years ago by aschilling

Replying to jpflori:

...as far as thematic tutos links were concerned.

I am confused what you are saying. I pulled the branch to my computer and ran make. Then I look at the compiled documentary. On the page

sage/src/doc/output/html/en/reference/combinat/sage/combinat/crystals/__init__.html

on my computer, the link to The Lie Methods and Related Combinatorics thematic tutorial for example seems broken.

Also, on the front page sage/src/doc/output/html/en/reference/combinat/index.html the title "Alphabetical Module List" should probably just be changed to "Module List".

I like the Search page :-)

comment:95 in reply to: ↑ 94 Changed 5 years ago by jpflori

Replying to aschilling:

Replying to jpflori:

...as far as thematic tutos links were concerned.

I am confused what you are saying. I pulled the branch to my computer and ran make. Then I look at the compiled documentary. On the page

sage/src/doc/output/html/en/reference/combinat/sage/combinat/crystals/__init__.html

on my computer, the link to The Lie Methods and Related Combinatorics thematic tutorial for example seems broken.

Ok, i'm also confused now. And I indeed rebroke something I actually fixed. The next commit I'll push should be ok.

Also, on the front page sage/src/doc/output/html/en/reference/combinat/index.html the title "Alphabetical Module List" should probably just be changed to "Module List".

Oh indeed, I just changed the title in module_list.rst itself.

I'm really not used to the doc structure yet.

I like the Search page :-)

comment:96 Changed 5 years ago by nthiery

  • Branch changed from u/jpflori/ticket/16256 to u/nthiery/ticket/16256

comment:97 Changed 5 years ago by nthiery

  • Commit changed from 7b753a2c64af971fc37d1fffa8f95e6fac1e0438 to 80d54890d91a024dfdd45a6d3eed4b8410dc25b5
  • Description modified (diff)

New commits:

80d548916256: Module List -> Comprehensive Module List and link to trac ticket

comment:98 Changed 5 years ago by git

  • Commit changed from 80d54890d91a024dfdd45a6d3eed4b8410dc25b5 to f5aa239b4946392411d97f4269ef5e646e215bfc

Branch pushed to git repo; I updated commit sha1. New commits:

f5aa239#16256: fixed links to thematic tutorials

comment:99 Changed 5 years ago by nthiery

Hi!

Thanks so much Jean-Pierre and Anne for your work on finalizing this, and sorry for reacting so late. I have checked all your changes, and am ok with them. I have created #17241, and linked to it from the doc, made "Module List" into "Comprehensive Module List", and rephrased a bit the description.

The version on the web was indeed outdated. I reupdated it.

Thanks Anne for spoting the broken thematic tutorial link in crystals.py. I spotted a couple others and fixed them.

For the various stylistic issues that were discussed those last days: if I am not mistaken, they are all referenced in the ticket description, and I agree that they would be best postponned to later tickets. They are indeed more about style than content, and involve more Sphinx programming/configuration rather than editing of the documentation sources.

If we all agree on this, then I believe it just remains to run the docbuilding with `--warn-links` and fix potential glitches. I'll do that now and report.

Cheers,

Nicolas


New commits:

f5aa239#16256: fixed links to thematic tutorials

comment:100 Changed 5 years ago by nthiery

For the record, below is what I got when checking out to master and back to this ticket, and running:

sage -docbuild reference/combinat html --warn-links 2>&1 | tee log

If I am not mistaken, those are all unrelated to this ticket (please double check), other than being in files that happen to be touched by this ticket. They thus should certainly be fixed at some point, but elsewhere.

What do you think? Ready to go?

Cheers,

Nicolas

[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/partition.py:docstring of sage.combinat.partition.Partition.to_dyck_word:5: WARNING: py:meth reference target not found: sage.combinat.dyck_word.DyckWord.to_partition
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/posets/posets.py:docstring of sage.combinat.posets.posets:1: WARNING: py:meth reference target not found: linear_extension
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/posets/posets.py:docstring of sage.combinat.posets.posets.FinitePoset:26: WARNING: py:class reference target not found: sage.combinat.posets.posets.PosetElement
[combinat ] :1: WARNING: py:class reference target not found: PosetElement
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/posets/posets.py:docstring of sage.combinat.posets.posets.FinitePoset.is_linear_extension:34: WARNING: py:class reference target not found: sage.combinat.posets.linear_extensions.LinearExtensionsOfPosets
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/posets/posets.py:docstring of sage.combinat.posets.posets.Poset:59: WARNING: py:class reference target not found: sage.combinat.posets.posets.PosetElement
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/cartan_type.py:docstring of sage.combinat.root_system.cartan_type:205: WARNING: py:class reference target not found: cached_method
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/cartan_type.py:docstring of sage.combinat.root_system.cartan_type:211: WARNING: py:meth reference target not found: set_mutable
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/cartan_type.py:docstring of sage.combinat.root_system.cartan_type.CartanType:207: WARNING: py:class reference target not found: cached_method
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/cartan_type.py:docstring of sage.combinat.root_system.cartan_type.CartanType:213: WARNING: py:meth reference target not found: set_mutable
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/cartan_type.py:docstring of sage.combinat.root_system.cartan_type.CartanTypeFactory.global_options:4: WARNING: py:obj reference target not found: CartanType.global_options
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/cartan_type.py:docstring of sage.combinat.root_system.cartan_type.CartanType_abstract:5: WARNING: py:meth reference target not found: dynkin_diagram
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/cartan_type.py:docstring of sage.combinat.root_system.cartan_type.CartanType_abstract.global_options:4: WARNING: py:obj reference target not found: CartanType.global_options
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/cartan_type.py:docstring of sage.combinat.root_system.cartan_type.CartanType_standard_affine.special_node:1: WARNING: py:meth reference target not found: CartanType_abstract.special_node
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/cartan_type.py:docstring of sage.combinat.root_system.cartan_type.CartanType_standard_finite.opposition_automorphism:8: WARNING: py:class reference target not found: Family
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/pieri_factors.py:docstring of sage.combinat.root_system.pieri_factors.PieriFactors.elements:14: WARNING: py:meth reference target not found: maximal_elements
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/pieri_factors.py:docstring of sage.combinat.root_system.pieri_factors.PieriFactors_affine_type:27: WARNING: py:meth reference target not found: _populate_generators_
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/pieri_factors.py:docstring of sage.combinat.root_system.pieri_factors.PieriFactors_affine_type:37: WARNING: py:meth reference target not found: Sets.Facade.ParentMethods.facade_for
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/pieri_factors.py:docstring of sage.combinat.root_system.pieri_factors.PieriFactors_affine_type:45: WARNING: py:meth reference target not found: _element_constructor
[combinat ] /opt/sage-git/src/doc/en/reference/combinat/sage/combinat/root_system/pieri_factors.rst:11: WARNING: py:meth reference target not found: _element_constructor_
[combinat ] /opt/sage-git/src/doc/en/reference/combinat/sage/combinat/root_system/pieri_factors.rst:5: WARNING: py:meth reference target not found: _repr_
[combinat ] /opt/sage-git/src/doc/en/reference/combinat/sage/combinat/root_system/pieri_factors.rst:14: WARNING: py:meth reference target not found: _repr_
[combinat ] /opt/sage-git/src/doc/en/reference/combinat/sage/combinat/root_system/pieri_factors.rst:7: WARNING: py:meth reference target not found: Parent.__init__
[combinat ] /opt/sage-git/src/doc/en/reference/combinat/sage/combinat/root_system/pieri_factors.rst:7: WARNING: py:mod reference target not found: sage.matrix.matrix_space.MatrixSpace
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/pieri_factors.py:docstring of sage.combinat.root_system.pieri_factors.PieriFactors_affine_type.maximal_elements:3: WARNING: py:meth reference target not found: _test_maximal_elements
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/root_system.py:docstring of sage.combinat.root_system.root_system.RootSystem:230: WARNING: py:mod reference target not found: sage.combinat.root_system
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/root_system.py:docstring of sage.combinat.root_system.root_system.RootSystem:231: WARNING: py:class reference target not found: RootSpace
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/root_system.py:docstring of sage.combinat.root_system.root_system.RootSystem:232: WARNING: py:class reference target not found: WeightSpace
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/root_system.py:docstring of sage.combinat.root_system.root_system.RootSystem:233: WARNING: py:class reference target not found: AmbientSpace
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/root_system.py:docstring of sage.combinat.root_system.root_system.RootSystem.ambient_space:30: WARNING: py:class reference target not found: sage.combinat.root_system.ambient_space.type_affine.AmbientSpace
[combinat ] /opt/sage-git/local/lib/python2.7/site-packages/sage/combinat/root_system/root_system.py:docstring of sage.combinat.root_system.root_system.RootSystem.ambient_space:32: WARNING: py:meth reference target not found: weight:space
[combinat ] /opt/sage-git/src/doc/en/reference/combinat/sage/combinat/words/__init__.rst:15: WARNING: py:class reference target not found: sage.combinat.words.shuffle_product.ShuffleProduct
[combinat ] /opt/sage-git/src/doc/en/reference/combinat/sage/combinat/words/__init__.rst:15: WARNING: py:class reference target not found: sage.combinat.words.shuffle_product.ShuffleProduct

comment:101 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:102 Changed 5 years ago by aschilling

  • Reviewers set to Anne Schilling
  • Status changed from needs_review to positive_review

comment:103 Changed 5 years ago by aschilling

Looks good to me now! I hope someone can make progress on #17421 since doing module or method lists by hand (as seems to be done in posets) is cumbersome and prone to error.

comment:104 follow-up: Changed 5 years ago by ncohen

There are fewer things in the Poset page than there used to be

-   ../sage/combinat/posets/posets
-   ../sage/combinat/posets/hasse_diagram
-   ../sage/combinat/posets/elements
-   ../sage/combinat/posets/lattices
-   ../sage/combinat/posets/linear_extensions
-   ../sage/combinat/posets/poset_examples
+- :ref:`sage.combinat.posets.posets`
+- Posets constructors: :func:`Poset`, :func:`MeetSemilattice`, :func:`JoinSemilattice`, :func:`LatticePoset`
+- Posets categories: :class:`~sage.categories.posets.Posets` and :class:`~sage.categories.lattice_posets.LatticePosets`
+- :class:`~sage.combinat.posets.linear_extensions.LinearExtensionOfPoset`, :class:`sage.combinat.posets.linear_extensions.LinearExtensionsOfPoset`
+
+- Catalog of posets: :obj:`Posets`
+- :ref:`sage.combinat.tamari_lattices`
+- :ref:`sage.combinat.interval_posets`

In particular Hasse Diagram has been removed. More importantly, the "poset_examples" file has been removed, and it is the one which contains the index of all pre-implemented Posets !

Nathann

comment:105 in reply to: ↑ 104 Changed 5 years ago by aschilling

Replying to ncohen:

There are fewer things in the Poset page than there used to be

-   ../sage/combinat/posets/posets
-   ../sage/combinat/posets/hasse_diagram
-   ../sage/combinat/posets/elements
-   ../sage/combinat/posets/lattices
-   ../sage/combinat/posets/linear_extensions
-   ../sage/combinat/posets/poset_examples
+- :ref:`sage.combinat.posets.posets`
+- Posets constructors: :func:`Poset`, :func:`MeetSemilattice`, :func:`JoinSemilattice`, :func:`LatticePoset`
+- Posets categories: :class:`~sage.categories.posets.Posets` and :class:`~sage.categories.lattice_posets.LatticePosets`
+- :class:`~sage.combinat.posets.linear_extensions.LinearExtensionOfPoset`, :class:`sage.combinat.posets.linear_extensions.LinearExtensionsOfPoset`
+
+- Catalog of posets: :obj:`Posets`
+- :ref:`sage.combinat.tamari_lattices`
+- :ref:`sage.combinat.interval_posets`

In particular Hasse Diagram has been removed. More importantly, the "poset_examples" file has been removed, and it is the one which contains the index of all pre-implemented Posets !

Hi Nathann, I found the pre-implemented posets under

sage/src/doc/output/html/en/reference/combinat/sage/combinat/posets/poset_examples.html linked under Catalog of posets: Posets. Is this not good enough?

Best,

Anne

comment:106 follow-up: Changed 5 years ago by ncohen

Hi Nathann, I found the pre-implemented posets under

Oh I see. I read "Posets" and I was thinking of

sage: Posets(4)
Posets containing 4 vertices

I did not know that "Posets" was equal to 'posets'.

But really I do not understand why you point toward the Poset function instead of the posets file, which shortcuts the module-level documentation. Users will miss the whole index of functions, won't they ?

http://www.sagemath.org/doc/reference/combinat/sage/combinat/posets/posets.html

Sorry I can't experiment with your branch, the University where I am has some stupid proxy rules and I cannot download a git branch.

Also, you did not mention Hasse Diagrams.

Nathann

comment:107 Changed 5 years ago by ncohen

  • Status changed from positive_review to needs_info

comment:108 in reply to: ↑ 106 Changed 5 years ago by nthiery

Hi Nathann,

Thanks for your double checking of posets!

Replying to ncohen:

But really I do not understand why you point toward the Poset function instead of the posets file, which shortcuts the module-level documentation. Users will miss the whole index of functions, won't they ?

Yes this module is important! The first item in the list points to it. The second item then points to the main constructors in this module.

Also, you did not mention Hasse Diagrams.

Yes because this is about the internal data structure of posets; it seems thus unlikely (in fact undesirable) that the user plays with it. Note that it appears in the comprehensive module list.

I could add it back if you believe it's important.

Sorry I can't experiment with your branch, the University where I am has some stupid proxy rules and I cannot download a git branch.

Yikes ...

You can find the result for this page on:

http://sage.math.washington.edu/home/nthiery/sage/src/doc/output/html/en/reference/combinat/sage/combinat/posets/__init__.html

Cheers,

Nicolas

comment:109 Changed 5 years ago by nthiery

  • Status changed from needs_info to needs_review

comment:110 Changed 5 years ago by ncohen

Yo !

Yes this module is important! The first item in the list points to it. The second item then points to the main constructors in this module.

Arg, I missed that.

Yes because this is about the internal data structure of posets; it seems thus unlikely (in fact undesirable) that the user plays with it. Note that it appears in the comprehensive module list.

Oh. So it is indeed included in the doc. That means that if the doc of some Poset functions links a HasseDiagram function the link will work, does it ?

I could add it back if you believe it's important.

If the links work that's fine. Indeed we don't want anybody playing with HasseDiagram directly.

Sorry I can't experiment with your branch, the University where I am has some stupid proxy rules and I cannot download a git branch.

Yikes ...

Yeah, plus I need to login 10 times each time I want to post a comment, hoping that I did not logout during the 3 seconds it takes top paste my text and click on 'submit'

http://sage.math.washington.edu/home/nthiery/sage/src/doc/output/html/en/reference/combinat/sage/combinat/posets/__init__.html

Hmmm... It is me or is that 10x more unclear than the current page:

http://www.sagemath.org/doc/reference/combinat/posets.html

Currently if a guy just wants to know what Posets can do he will click on "Posets", and no other link makes any sense. Now among the links you have "Finite Posets (module), Poset (constructor), Posets (category), Poset (catalog)".

I will add a commit this evening, or as soon as I can get 'git' to work.

Nathann

comment:111 follow-ups: Changed 5 years ago by ncohen

Hello !

I added a commit at public/16256 which does the following:

  • Split the links of the current posets page into three parts: 1) the catalog 2) the main classes 3) the categories
  • I transformed the class links into ref links, this way the users will reach the module's doc whose purpose is precisely to explain what is inside (with an index of functions whenever possible)
  • The 'categories' links appears a bit like a footnote now, as it really isn't the right folder and those links are here to redirect lost readers.
  • Change "The family of Generalized Tamari Lattices" into 'Generalized Tamari Lattices'
  • Added 'Combinatorial' in front of 'designs', as it is their most common name.

I will create another patch (should be done in 30 minutes) that:

  • Creates an index of poset constructors in poset_examples (that was the aim of pointing toward the module in the first place rather than to the 'posets' object)
  • I will mention the two classes contained in linear_extensions in this module's doc, as I changed the two links into one in this patch.

I only looked at what this branch changed in the poset and design doc, so if there is no problem with this last commit the ticket can be set back to positive_review on Anne's behalf.

Nathann

comment:112 in reply to: ↑ 111 Changed 5 years ago by ncohen

I will create another patch (should be done in 30 minutes) that:

Done at #17424.

Nathann

comment:113 in reply to: ↑ 111 ; follow-up: Changed 5 years ago by nthiery

Replying to ncohen:

I added a commit at public/16256 which does the following:

  • Split the links of the current posets page into three parts: 1) the catalog 2) the main classes 3) the categories
  • I transformed the class links into ref links, this way the users will reach the module's doc whose purpose is precisely to explain what is inside (with an index of functions whenever possible)
  • The 'categories' links appears a bit like a footnote now, as it really isn't the right folder and those links are here to redirect lost readers.
  • Change "The family of Generalized Tamari Lattices" into 'Generalized Tamari Lattices'
  • Added 'Combinatorial' in front of 'designs', as it is their most common name.

Ok for me.

Note: People using posets a bit seriously need to know about the existence of the categories to have an idea of where the available methods come from. So the links are not just about "lost readers". I am fine with the layout you propose though.

I will create another patch (should be done in 30 minutes) that:

  • Creates an index of poset constructors in poset_examples (that was the aim of pointing toward the module in the first place rather than to the 'posets' object)
  • I will mention the two classes contained in linear_extensions in this module's doc, as I changed the two links into one in this patch.

Thanks.

Cheers,

Nicolas

comment:114 Changed 5 years ago by git

  • Commit changed from f5aa239b4946392411d97f4269ef5e646e215bfc to a5208fd118f578f5f36a57caaf43e6a81e48653b

Branch pushed to git repo; I updated commit sha1. New commits:

a5208fdtrac #16256: Rewrite the posets doc page

comment:115 Changed 5 years ago by nthiery

  • Status changed from needs_review to positive_review

New commits:

a5208fdtrac #16256: Rewrite the posets doc page

comment:116 in reply to: ↑ 113 ; follow-up: Changed 5 years ago by ncohen

Note: People using posets a bit seriously need to know about the existence of the categories to have an idea of where the available methods come from. So the links are not just about "lost readers". I am fine with the layout you propose though.

As far as the documentation is concerned, we should have all functions appear in the poset index, even those that are implemented somewhere else. There is no systematic way to 'guess' in which file a Poset function should be implemented, so we must keep it simple at least on the user side.

When it comes to the developper's side, it would also be nice if having categories did not mean having to move functions where they do not belong. The 'categories/' folder should only contain category code, not Poset-specific code. Not any kind of mathematical code, actually.

Or perhaps we should move the categories/poset file to combinat/posets ? That would make more sense to me.

Nathann

comment:117 in reply to: ↑ 116 Changed 5 years ago by nthiery

Replying to ncohen:

As far as the documentation is concerned, we should have all functions appear in the poset index, even those that are implemented somewhere else.

Sure. Even more, this should be automatized at some point. But we are deviating topic.

There is no systematic way to 'guess' in which file a Poset function should be implemented:

It's not so bad, for a developer at least: the question is just whether the implementation of the function depend on the specific data structure or not. Well, I am going off topic again.

so we must keep it simple at least on the user side.

Sure.

When it comes to the developper's side, it would also be nice if having categories did not mean having to move functions where they do not belong. The 'categories/' folder should only contain category code, not Poset-specific code. Not any kind of mathematical code, actually.

Or perhaps we should move the categories/poset file to combinat/posets ? That would make more sense to me.

Well, so far the convention, in Sage and in other software with similar mechanisms, has been to keep all the general purpose mathematical categories together. This helps keeping them consistent with each other, keeping an overview of the hierarchy, etc. This could be changed, but this should be done consistently.

Cheers,

Nicolas

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

  • Status changed from positive_review to needs_work

pdf docs don't build.

comment:119 in reply to: ↑ 118 Changed 5 years ago by nthiery

Replying to vbraun:

pdf docs don't build.

Oops, I should have tested that as it had some chances to fail since this tickets adds some Python files to the reference manual. Thanks for reporting.

I am on it.

comment:120 Changed 5 years ago by git

  • Commit changed from a5208fd118f578f5f36a57caaf43e6a81e48653b to c8108f8b96b33fcedbb22e13f331e3f2ed99ab05

Branch pushed to git repo; I updated commit sha1. New commits:

c8108f816256: fixed ReST typo in newly included file preventing PDF doc compilation

comment:121 Changed 5 years ago by nthiery

pdf-doc still not compiling here, but that's now for a "Cannot allocate memory". I am now running the compilation on another machine with larger memory.

comment:122 Changed 5 years ago by nthiery

  • Status changed from needs_work to needs_review

Worked. Back to needs review for the last trivial commit!

comment:123 Changed 5 years ago by vbraun

  • Status changed from needs_review to positive_review

comment:124 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work

Internet tests must be optional:

sage -t --long src/sage/combinat/quickref.py
**********************************************************************
File "src/sage/combinat/quickref.py", line 7, in sage.combinat.quickref
Failed example:
    s = oeis([1,3,19,211]); s
Exception raised:
    Traceback (most recent call last):
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 488, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 851, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.combinat.quickref[0]>", line 1, in <module>
        s = oeis([Integer(1),Integer(3),Integer(19),Integer(211)]); s
      File "sage/misc/lazy_import.pyx", line 358, in sage.misc.lazy_import.LazyImport.__call__ (build/cythonized/sage/misc/lazy_import.c:3230)
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/databases/oeis.py", line 372, in __call__
        return self.find_by_subsequence(query, max_results, first_result)
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/databases/oeis.py", line 500, in find_by_subsequence
        return self.find_by_description(subsequence, max_results, first_result)
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/databases/oeis.py", line 465, in find_by_description
        sequence_list = _fetch(url).split('\n\n')[2:-1]
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/databases/oeis.py", line 189, in _fetch
        raise IOError("%s\nError fetching %s." % (msg, url))
    IOError: [Errno socket error] [Errno 60] Operation timed out
    Error fetching http://oeis.org/search?q=1%2C+3%2C+19%2C+211&start=0&fmt=text&n=3.
**********************************************************************

comment:125 Changed 5 years ago by git

  • Commit changed from c8108f8b96b33fcedbb22e13f331e3f2ed99ab05 to 7fe79fb6ec8b72e31db862d018c142a77515a0eb

Branch pushed to git repo; I updated commit sha1. New commits:

7fe79fb16256: made oeis tests optional (depend on internet)

comment:126 Changed 5 years ago by nthiery

  • Reviewers changed from Anne Schilling to Anne Schilling, Nathann Cohen
  • Status changed from needs_work to needs_review

Gasp! Out of curiosity, how did you catch this? You have a no-internet test bot?

Thanks!

comment:127 Changed 5 years ago by vbraun

  • Status changed from needs_review to positive_review

All buildbot slaves poison various proxy environment variables...

comment:128 Changed 5 years ago by nthiery

Cool :-) Thanks!

comment:129 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work

More failures:

sage -t --long --warn-long 47.3 src/sage/combinat/quickref.py
**********************************************************************
File "src/sage/combinat/quickref.py", line 9, in sage.combinat.quickref
Failed example:
    s[0].programs()
Exception raised:
    Traceback (most recent call last):
      File "/mnt/disk/home/release/Sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 488, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/mnt/disk/home/release/Sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 851, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.combinat.quickref[0]>", line 1, in <module>
        s[Integer(0)].programs()
    NameError: name 's' is not defined
**********************************************************************
File "src/sage/combinat/quickref.py", line 61, in sage.combinat.quickref
Failed example:
    Sym = SymmetricFunctions(QQ); Sym.inject_shorthands()
Expected:
    doctest:...: RuntimeWarning: redefining global value `s`
    doctest:...: RuntimeWarning: redefining global value `e`
Got:
    doctest:1198: RuntimeWarning: redefining global value `e`
**********************************************************************

comment:130 Changed 5 years ago by vbraun

Also conflicts with #16018 which I just merged.

comment:131 Changed 5 years ago by ncohen

  • Reviewers changed from Anne Schilling, Nathann Cohen to Anne Schilling

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

comment:132 follow-up: Changed 5 years ago by kcrisman

  • Reviewers changed from Anne Schilling to Anne Schilling, Nathann Cohen

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).

comment:133 in reply to: ↑ 132 Changed 5 years ago by ncohen

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

comment:134 Changed 5 years ago by jpflori

  • Branch changed from u/nthiery/ticket/16256 to u/jpflori/ticket/16256
  • Commit changed from 7fe79fb6ec8b72e31db862d018c142a77515a0eb to 823c116a5e575ddcbadaba0075ac00fec245efba
  • Status changed from needs_work to positive_review

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.

comment:135 Changed 5 years ago by nthiery

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 :-)

comment:136 Changed 5 years ago by vbraun

  • Branch changed from u/jpflori/ticket/16256 to 823c116a5e575ddcbadaba0075ac00fec245efba
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:137 Changed 5 years ago by ncohen

  • Commit 823c116a5e575ddcbadaba0075ac00fec245efba deleted

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

comment:138 Changed 5 years ago by nthiery

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

comment:139 Changed 5 years ago by ncohen

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

comment:140 Changed 5 years ago by ncohen

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

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

comment:141 Changed 5 years ago by nthiery

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,

comment:142 Changed 5 years ago by ncohen

This has been fixed as part of #17634.

Nathann

comment:143 follow-up: Changed 3 years ago by 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.

comment:144 in reply to: ↑ 143 Changed 3 years ago by nthiery

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.

Note: See TracTickets for help on using tickets.