#16911 closed enhancement (fixed)
Update sagenb
Reported by: | Punarbasu Purkayastha | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.4 |
Component: | notebook | Keywords: | |
Cc: | Karl-Dieter Crisman, Julien Puydt, François Bissey | Merged in: | |
Authors: | Punarbasu Purkayastha | Reviewers: | Julien Puydt, Karl-Dieter Crisman |
Report Upstream: | N/A | Work issues: | |
Branch: | b84d502 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | #16396 | Stopgaps: |
Description (last modified by )
This ticket is for
- updating sagenb. Corresponding upstream ticket is https://github.com/sagemath/sagenb/pull/218
- updating documentation inside sage to reflect the new development model for sagenb
cd SAGE_ROOT/upstream wget -c 'https://googledrive.com/host/0B2kYdLdBVFHIZEd4ekpjNUFBNTQ/sagenb-0.10.8.3.tar' git checkout -b develop ticket/16911 git trac pull 16911 ../sage -i sagenb ../sage -b
To @vbraun: When merging this ticket, merge both this and #16396 since sagenb upstream has changes to fix doctests in that ticket, and that ticket in turn depends on an updated sagenb so that it can pass all doctests. Also close #11469 as that is incorporated here.
Change History (47)
comment:1 Changed 8 years ago by
Branch: | → u/ppurka/update_sagenb |
---|
comment:2 Changed 8 years ago by
Cc: | Karl-Dieter Crisman added |
---|---|
Commit: | → 2d3d3e993bb1bef1f25fcf36dcd45cbc37d3c132 |
comment:3 Changed 8 years ago by
Cc: | Julien Puydt added |
---|
comment:4 Changed 8 years ago by
Commit: | 2d3d3e993bb1bef1f25fcf36dcd45cbc37d3c132 → fe9630cb38206b159300438adb3fa552090f8acf |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
fe9630c | update sagenb version and checksum
|
comment:5 Changed 8 years ago by
Description: | modified (diff) |
---|
Updated to new version with the upstream sagenb changes made to sphinx. The method to check it out is in description.
comment:6 Changed 8 years ago by
Status: | new → needs_review |
---|
comment:7 Changed 8 years ago by
Status: | needs_review → needs_work |
---|
Hmmmm... of course it fails to build since it's looking for upstream sources which we don't have... how shall I test this ticket and #16396 without it?
comment:8 Changed 8 years ago by
Installation instructions are in the ticket description. If you don't have wget then you can download the file manually from Google drive.
comment:9 Changed 8 years ago by
Cc: | François Bissey added |
---|
comment:10 Changed 8 years ago by
Status: | needs_work → positive_review |
---|
Sigh... and I had actually checked if it wasn't in the description!
It compiles and passes all doctests. I haven't yet looked at what it gives with #16396, but for this ticket, it's a positive review.
comment:11 Changed 8 years ago by
Status: | positive_review → needs_work |
---|
Thanks. It is not complete yet. I wanted to put it up here so that you can test your ticket.
Todo:
- merge upstream ticket about documentation, and
- update change log.
comment:12 Changed 8 years ago by
I have checked that the ticket as-is passes all doctests, and makes #16396 also pass everything.
comment:13 Changed 8 years ago by
Thanks for checking! Have you played with the notebook itself to make sure there aren't any weird regressions (like the notebook not working...) that we missed?
comment:14 Changed 8 years ago by
Hmmmm... I hadn't used the notebook since long, but I just failed to get a plot! Is there something wrong with:
from pylab import * xs=linspace(0,6) ys=sin(xs) plot(xs,ys) show()
?
comment:15 follow-up: 16 Changed 8 years ago by
I'm not sure that ever worked. A figure has to be saved for it to show up, and I don't know that matplotlib does that.
savefig('hi.png')
should work, however - try that.
comment:16 Changed 8 years ago by
I'm not sure that ever worked. A figure has to be saved for it to show up, and I don't know that matplotlib does that.
savefig('hi.png')should work, however - try that.
To be fair, in the Ipython notebook I assume that your code does work :)
comment:18 follow-up: 21 Changed 8 years ago by
Though notice that I have the following in the logs:
/home/jpuydt/sage.git/local/lib/python2.7/site-packages/Crypto/Util/number.py:57: PowmInsecureWarning: Not using mpz_powm_sec. You should rebuild using libgmp >= 5 to avoid timing attack vulnerability. _warn("Not using mpz_powm_sec. You should rebuild using libgmp >= 5 to avoid timing attack vulnerability.", PowmInsecureWarning)
comment:19 Changed 8 years ago by
That is known and unrelated. You can try it in a Sage without this and probably you'll see the same thing. I do in all my installs.
But as ppurka says here, there remains a little cosmetic work.
comment:21 Changed 8 years ago by
Replying to Snark:
Though notice that I have the following in the logs:
/home/jpuydt/sage.git/local/lib/python2.7/site-packages/Crypto/Util/number.py:57: PowmInsecureWarning: Not using mpz_powm_sec. You should rebuild using libgmp >= 5 to avoid timing attack vulnerability. _warn("Not using mpz_powm_sec. You should rebuild using libgmp >= 5 to avoid timing attack vulnerability.", PowmInsecureWarning)
Independent from this ticket. We have already seen that if I remember correctly and it won't go away for some time while we are using mpir to provide gmp.
comment:22 Changed 8 years ago by
I'm happy with the instructions on the Sage side as well, or at least happier :)
comment:23 Changed 8 years ago by
By the way, ppurka, there are a few other ready or almost-ready things that are actually fairly important upstream by migurehito, so if they come quick we should probably merge them too (things about admin errors).
comment:25 Changed 8 years ago by
Commit: | fe9630cb38206b159300438adb3fa552090f8acf → b84d5029fc38e5882d6e589481dbe17e4452860e |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
b84d502 | Update checksums for sagenb 0.10.8.3
|
comment:26 Changed 8 years ago by
Authors: | → Punarbasu Purkayastha |
---|---|
Reviewers: | → Julien Puydt, Karl-Dieter Crisman |
Status: | needs_work → positive_review |
Ok. I have updated sagenb upstream, the sagenb.tar, and added the new checksum. This is good to go. Setting it to positive review.
comment:27 Changed 8 years ago by
Dependencies: | → #16396 |
---|
comment:28 Changed 8 years ago by
comment:29 Changed 8 years ago by
comment:30 Changed 8 years ago by
comment:31 Changed 8 years ago by
comment:32 Changed 8 years ago by
Just FYI, François has updated #16396 and Julien seems to have confirmed it works, just need for that one to have the one last check and then we're good to go here.
comment:33 Changed 8 years ago by
Description: | modified (diff) |
---|
comment:34 Changed 8 years ago by
Branch: | u/ppurka/update_sagenb → b84d5029fc38e5882d6e589481dbe17e4452860e |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
comment:35 Changed 8 years ago by
Commit: | b84d5029fc38e5882d6e589481dbe17e4452860e |
---|
How can this ticket be fixed without the following?
To @vbraun: When merging this ticket, merge both this and #16396 since sagenb upstream has changes to fix doctests in that ticket, and that ticket in turn depends on an updated sagenb so that it can pass all doctests. Also close #11469 as that is incorporated here.
comment:36 follow-up: 37 Changed 8 years ago by
I merged #16396. Isn't that sufficient? Please please put dependencies in the dependency field, I can't read through every ticket description / discussion when merging.
comment:37 Changed 8 years ago by
I merged #16396. Isn't that sufficient?
I think at the time it hadn't been merged, though I'm sure it was in your queue at that point.
Please please put dependencies in the dependency field, I can't read through every ticket description / discussion when merging.
Yes, and the sagenb tickets aren't the only ones where this happens. Perhaps a gentle reminder on sage-devel would be helpful, I'll do that.
comment:38 follow-ups: 39 43 Changed 8 years ago by
Was sagenb 0.11.0 merged as part of this ticket? For what it's worth, the tarball was not prepared correctly: it has two copies of most of the component pieces (like Babel-1.3.tar.gz
). When you unpack it, it has at the top level another tarball called sagenb-0.11.0.tar.gz
which contains a second copy of everything. This is why the "upstream" tarball is twice the size of the one for sagenb-0.10.8.2.
comment:39 Changed 8 years ago by
Was sagenb 0.11.0 merged as part of this ticket?
It wasn't supposed to be. That would have been #16004.
For what it's worth, the tarball was not prepared correctly: it has two copies of most of the component pieces (like
Babel-1.3.tar.gz
). When you unpack it, it has at the top level another tarball calledsagenb-0.11.0.tar.gz
which contains a second copy of everything. This is why the "upstream" tarball is twice the size of the one for sagenb-0.10.8.2.
I just followed the instructions at https://gist.github.com/ppurka/d10f668db113c2d1ddd4 as recommended. I wouldn't be surprised if I did something wrong, though.
I'd be happy to try to fix that but would need very explicit instructions; I don't want to mess around trying to guess the right thing to do.
comment:40 follow-up: 42 Changed 8 years ago by
I think that it can probably wait until the next sagenb update. By the way, I just noticed that since sagenb-0.10.8.2, I see the following message in the installation log file:
Note: Bypassing http://github.com/mitsuhiko/flask-oldsessions/tarball/master#egg=flask-oldsessions-0.10 (disallowed host; see http://bit.ly/1dg9ijs for details).
What does this mean?
comment:41 Changed 8 years ago by
I also noticed that. I believe it means we are supposed to download (see e.g. https://github.com/sagemath/sagenb/blob/master/util/fetch_deps.py) that but didn't because somewhere we don't allow this host. I don't know exactly where we use easy_install
, though, so I couldn't easily debug that.
comment:42 Changed 8 years ago by
I think that it can probably wait until the next sagenb update.
That may come sooner than you think; see #17292.
comment:43 Changed 8 years ago by
Was sagenb 0.11.0 merged as part of this ticket? For what it's worth, the tarball was not prepared correctly: it has two copies of most of the component pieces (like
Babel-1.3.tar.gz
). When you unpack it, it has at the top level another tarball calledsagenb-0.11.0.tar.gz
which contains a second copy of everything. This is why the "upstream" tarball is twice the size of the one for sagenb-0.10.8.2.
Looks like we will indeed be releasing a new one, so if you have an easy fix for this, I would appreciate that - you can communicate at https://github.com/sagemath/sagenb/issues or here or at #17292 or in private communication.
comment:44 follow-up: 46 Changed 8 years ago by
I don't have an easy fix. It turns out that the big tarball contained a smaller tarball sagenb-0.11.0.tar.gz
which actually was just the right thing: you could just keep the smaller one and use it in the upstream directory. I guess I would say, keep an eye on the file sizes. If it's much bigger than the previous one (for 0.10.8.2 or whatever it was), then something might be wrong.
comment:45 Changed 8 years ago by
Hmm, then I guess the problem is that the instructions have an extra line. Because python setup.py sdist already makes a .tar.gz
, and then we in addition do
tar cf sagenb-<version>.tar sagenb-<version>
which presumably leads to the tar file you found. So that step is probably extra.
comment:46 Changed 8 years ago by
I don't have an easy fix. It turns out that the big tarball contained a smaller tarball
sagenb-0.11.0.tar.gz
which actually was just the right thing: you could just keep the smaller one and use it in the upstream directory. I guess I would say, keep an eye on the file sizes. If it's much bigger than the previous one (for 0.10.8.2 or whatever it was), then something might be wrong.
I dunno, I'm not convinced now. I just repackaged a new one (it's not 100% ready, but just to test) and got
$ du sagenb-0.11.1.tar 35824 sagenb-0.11.1.tar $ du upstream/sagenb-0.10.8.2.tar 33224 upstream/sagenb-0.10.8.2.tar
Given that there is a new jQuery and other goodies, this seems like a small difference. I can confirm that the previous one was wrong, but I don't know how this happened. I'll check in the future, though.
comment:47 Changed 8 years ago by
I think that's probably okay. It's not twice the size, which is what happened before.
New commits:
update sagenb developer guide with new dev instructions