Opened 8 years ago

Closed 8 years ago

#14669 closed enhancement (fixed)

autogenerate the list of subdirectories of doc/en/reference

Reported by: jhpalmieri Owned by: mvngu
Priority: major Milestone: sage-5.11
Component: documentation Keywords:
Cc: vbraun Merged in: sage-5.11.beta0
Authors: John Palmieri Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

As the summary says. Currently, the file doc/en/reference/conf.py explicitly lists all of the subdirectories which contain components of the reference manual. If anyone adds a new component, they have to edit this file, and if they forget, they can get opaque error messages. So instead, the list should be generated on the fly.

Attachments (1)

trac_14669.patch (2.0 KB) - added by jhpalmieri 8 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 8 years ago by jhpalmieri

  • Status changed from new to needs_review

Slightly different from the patch at #7477: I've removed 'sage', 'sagenb', 'media', and 'other' from the list of directories -- these might be present if upgrading Sage.

comment:2 Changed 8 years ago by vbraun

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

Looks good to me.

comment:3 Changed 8 years ago by leif

Minor thing: The new list isn't sorted.

comment:4 Changed 8 years ago by leif

Stylewise: It would be better to create a set of the subdir names and afterwards remove the "bad" ones from that (with .difference_update()).

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

To the best of my knowledge neither the order of the entry nor the performance of this step of the doc build matter.

comment:6 in reply to: ↑ 5 Changed 8 years ago by leif

Replying to vbraun:

To the best of my knowledge neither the order of the entry nor the performance of this step of the doc build matter.

To you...

If the folders are sorted (or more precisely, get processed in alphabetical order), you at least get an idea of the progress.

W.r.t. performance: It certainly doesn't matter here (at least currently ;-) ), that's why I wrote stylewise. People* tend to get used to such, adding overhead here and there, and at some point it does matter.

(* which includes people reading such code, not necessarily their author(s))

comment:7 Changed 8 years ago by jhpalmieri

I don't mind sorting the list, but note that once you pass the list off to parallel processing, you tend to lose control of the ordering. For example, I usually see polynomial_rings build first. As far as style goes, there is frequently a trade-off between readability and performance. In this case, I think that just using lists is more readable, and since performance is not an issue, we should leave it as is.

comment:8 Changed 8 years ago by jhpalmieri

Difference between the old and new patch:

  • doc/en/reference/conf.py

    diff --git a/doc/en/reference/conf.py b/doc/en/reference/conf.py
    a b  
    6464
    6565multidocs_is_master = True
    6666
    67 # List of subdocs. Include all subdirectories of ref_src except for
    68 # 'static' and 'templates', and to deal with upgrades: 'sage',
     67# Sorted list of subdocs. Include all subdirectories of ref_src except
     68# for 'static' and 'templates', and to deal with upgrades: 'sage',
    6969# 'sagenb', 'media', and 'other'.
    7070bad_directories = ['static', 'templates', 'sage', 'sagenb', 'media', 'other']
    71 multidocs_subdoc_list = [x for x in os.listdir(ref_src) if
    72                          os.path.isdir(os.path.join(ref_src, x))
    73                          and x not in bad_directories]
     71multidocs_subdoc_list = [x for x in os.listdir(ref_src)
     72                         if os.path.isdir(os.path.join(ref_src, x))
     73                         and x not in bad_directories].sort()
    7474
    7575# List of directories, relative to source directory, that shouldn't be
    7676# searched for source files.

EDIT: the patch uses sorted([...]) rather than [...].sort(), of course.

Last edited 8 years ago by jhpalmieri (previous) (diff)

comment:9 Changed 8 years ago by leif

ahknst, KO.

Changed 8 years ago by jhpalmieri

comment:10 follow-up: Changed 8 years ago by vbraun

The docbuilder will sort the bags of work by decreasing number of subdirectories (approx. computational effort), so its pointless to sort here.

comment:11 Changed 8 years ago by jhpalmieri

I think that the value of multidocs_subdoc_list is part of the pickle stored for the documentation. So in the unlikely event that repeated execution of os.listdir(...) produces lists in different orders, we should sort it to make sure the pickle is consistent from one run to the next.

comment:12 in reply to: ↑ 10 ; follow-up: Changed 8 years ago by leif

Replying to vbraun:

The docbuilder will sort the bags of work by decreasing number of subdirectories (approx. computational effort),

... which again will maximize the (expected) peak memory usage.

comment:13 Changed 8 years ago by rbeezer

Step out for a few hours and you miss a lot. ;-)

I just used this to build the reference manual on 5.10.beta5 by nuking doc/output and doing

sage -docbuild reference inventory
sage -docbuild reference html

and it seemed to work just fine. At least the matroid documentation rendered fine. So I'm all for a positive review based on my testing. I'll stay out of the sorted discussion.

Rob

comment:14 in reply to: ↑ 12 Changed 8 years ago by vbraun

Replying to leif:

... which again will maximize the (expected) peak memory usage.

Exactly! You finally got the point of parallelism ;-) We are trying to use all resources of the computer at once. If you don't have enough memory or your IO is slow then you don't benefit from multiple processors.

comment:15 Changed 8 years ago by jdemeyer

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