-
-
Notifications
You must be signed in to change notification settings - Fork 453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make matplotlib support SAGE_SPKG_INSTALL_DOCS #10828
Comments
comment:2
A few points.
It looks like similar issues arise with the other updated .spkg files that you produced which are related to #10823 |
Attachment: 10828.patch.gz FYI Only---already applied to spkg. |
FYI Only---already applied to spkg. |
This comment has been minimized.
This comment has been minimized.
comment:3
Attachment: 10828-1.patch.gz |
comment:4
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:6
New spkg is up, ready for review. |
Reviewer: David Kirkby |
comment:7
This generates the HTML and puts it where expected. However, the simple
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 I think we need to find the unnecessary files and remove them. Something like:
would be a start, but there's probably more that can be done. Should we generate all the PDFs if 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
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
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. 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
I've posted a message to the matplotlib list about the .pyc files, as well as a possible redundancy in image files. |
comment:11
OK, in light of what you have said, I'll put this to positive review. |
comment:12
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 |
Merged: sage-4.7.alpha1 |
comment:14
matplotlib doesn't build from scratch using SAGE_SPKG_INSTALL_DOCS, see also #10826. |
Changed merged from sage-4.7.alpha1 to none |
comment:16
See #11197 for building docs after Sage is built. |
Changed keywords from none to sd32 |
Changed reviewer from David Kirkby to none |
Changed author from Jason Grout to none |
comment:22
outdated, should close |
Reviewer: Dima Pasechnik |
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)
CC: @sagetrac-ryan
Component: packages: standard
Keywords: sd32
Reviewer: Dima Pasechnik
Issue created by migration from https://trac.sagemath.org/ticket/10828
The text was updated successfully, but these errors were encountered: