Opened 9 years ago

Last modified 5 years ago

#10828 needs_work enhancement

Make matplotlib support SAGE_SPKG_INSTALL_DOCS

Reported by: jason Owned by: tbd
Priority: major Milestone: sage-6.4
Component: packages: standard Keywords: sd32
Cc: ryan Merged in:
Authors: Jason Grout Reviewers: David Kirkby
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jason)

This ticket implements #10823 for matplotlib. New spkg at  http://sage.math.washington.edu/home/jason/spkg-docs/matplotlib-1.0.1.p0.spkg (depends on #10588)

Attachments (2)

10828.patch (1.3 KB) - added by jason 9 years ago.
FYI Only---already applied to spkg.
10828-1.patch (980 bytes) - added by jason 9 years ago.
FYI Only---already applied to spkg.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 9 years ago by jason

  • Status changed from new to needs_review

comment:2 Changed 9 years ago by drkirkby

  • Status changed from needs_review to needs_work

A few points.

  • The SPKG.txt file does not have the number of this ticket in it - only #10823
  • On #10823 the patch which updates the Developers Guide shows the documentation being copied not moved, which is better, as it leaves the build directory untouched.
  • It would make reviewing much easier, and be a better historical record, if there was a Mercurial patch of the changes attached to the ticket.

It looks like similar issues arise with the other updated .spkg files that you produced which are related to #10823

Changed 9 years ago by jason

FYI Only---already applied to spkg.

Changed 9 years ago by jason

FYI Only---already applied to spkg.

comment:3 Changed 9 years ago by jason

  • Description modified (diff)

comment:4 Changed 9 years ago by jason

  • Cc ryan added

I also found that the last matplotlib spkg from #10588 had a dirty src/ directory, instead a pristine copy of the sources. That was probably all my fault, as I was the reviewer, and the contributor was a new contributor. I'm CCing Ryan to make sure he knows about it, just as an FYI.

The new matplotlib spkg above replaces the src/ directory with a pristine copy of the 1.0.1 matplotlib sources, which removes the build directory, some of the files that are generated in compilation, and trims the final spkg from 19M to 11M.

comment:5 Changed 9 years ago by jason

  • Status changed from needs_work to needs_review

comment:6 Changed 9 years ago by jason

New spkg is up, ready for review.

comment:7 Changed 9 years ago by drkirkby

  • Reviewers set to David Kirkby
  • Status changed from needs_review to needs_work

This generates the HTML and puts it where expected. However, the simple

cp -r doc/* $SAGE_ROOT/local/share/doc/PACKAGE_NAME/

is copying a lot of unnecessary files. There are 29 files with the extension .py, 511 files with the extension .pyc, and possibly some other unnecessary files that are being copied to $SAGE_ROOT/local/share/doc/PACKAGE_NAME/ There are 945 pdf files there too - I'm not quite sure how they got there. The total size of the files copied to $SAGE_ROOT/local/share/doc/PACKAGE_NAME/ is 115 MB

I think we need to find the unnecessary files and remove them. Something like:

find "$SAGE_ROOT/local/share/doc/matplotlib" -name '*.pyc' -exec rm -f {} \;
find "$SAGE_ROOT/local/share/doc/matplotlib" -name '*.py'  -exec rm -f {} \;

would be a start, but there's probably more that can be done.

Should we generate all the PDFs if pdftex is present? (In which case there will be several hundred more), or delete all the PDFs??

I can't see any point in having the .py and .pyc files, but I'm not so sure about the .pdf files.

I'm not going to copy the same comments to all the other packages - lets get one right first, before going on to the others.

I think #10823 should be updated to advise developers to remove unnecessary files if they are present, otherwise we are just going to be copying a lot of unneeded junk, and filling up disks unnecessarily.

Dave

comment:8 Changed 9 years ago by jason

In matplotlib, it's not "cp -r doc/* $SAGE_ROOT/local/share/doc/PACKAGE_NAME/", but rather, it's copying the results of the build directory.

The documentation for matplotlib includes a *lot* of example figures (generated in both png and pdf formats) as well as the .py files that generated those examples, so I think those files are legitimate. I'm not sure about the .pyc files---I'll check those.

comment:9 Changed 9 years ago by jason

  • Status changed from needs_work to needs_review

This spkg (as well as the other spkgs from #10823) are copying the directories containing the actual built documentation (e.g., build/html), not the entire documentation source tree. As such, only the actual documentation files should be there. In matplotlib's case, lots of pdf and png files are generated as example images (this is a plotting package, after all) and the generating .py files are also included in the docs.

I see some pyc files that probably should not be in the build directory of the docs. I feel like these should be reported upstream and dealt with on another ticket. I'd really rather use the default upstream doc build system, and report problems upstream.

Please remember that SAGE_SPKG_INSTALL_DOCS is an optional feature, and the people that invoke it are making a decision that the documentation is worth the disk space.

comment:10 Changed 9 years ago by jason

I've posted a message to the matplotlib list about the .pyc files, as well as a possible redundancy in image files.

comment:11 Changed 9 years ago by drkirkby

  • Status changed from needs_review to positive_review

OK, in light of what you have said, I'll put this to positive review.

comment:12 Changed 9 years ago by jason

An upstream developer has proposed some changes to matplotlib that will cut down on the redundant files. See the mailing list thread here: http://thread.gmane.org/gmane.comp.python.matplotlib.general/26576

comment:13 Changed 9 years ago by jdemeyer

  • Merged in set to sage-4.7.alpha1
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:14 Changed 8 years ago by jdemeyer

  • Merged in sage-4.7.alpha1 deleted
  • Resolution fixed deleted
  • Status changed from closed to new

matplotlib doesn't build from scratch using SAGE_SPKG_INSTALL_DOCS, see also #10826.

comment:15 Changed 8 years ago by jdemeyer

  • Status changed from new to needs_work

comment:16 Changed 8 years ago by jason

See #11197 for building docs after Sage is built.

comment:17 Changed 8 years ago by was

  • Keywords sd32 added

comment:18 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:19 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:20 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:21 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4
Note: See TracTickets for help on using tickets.