Opened 5 years ago
Closed 5 years ago
#23066 closed enhancement (fixed)
sagenb update to 1.0
Reported by: | dimpase | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.0 |
Component: | notebook | Keywords: | |
Cc: | jdemeyer, jhpalmieri, kcrisman | Merged in: | |
Authors: | Dima Pasechnik | Reviewers: | John Palmieri, Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | 7b85dc2 (Commits, GitHub, GitLab) | Commit: | 7b85dc25f339d0988761c22da2f3dd4a2edc94c4 |
Dependencies: | Stopgaps: |
Description (last modified by )
This is sagenb update, incorporating changes accumulating since May 2016.
Tarball: http://users.ox.ac.uk/~coml0531/sage/sagenb-1.0.1.tar.bz2
Change History (26)
comment:1 Changed 5 years ago by
- Cc jdemeyer jhpalmieri kcrisman added
- Status changed from new to needs_review
comment:2 follow-up: ↓ 3 Changed 5 years ago by
comment:3 in reply to: ↑ 2 Changed 5 years ago by
Replying to jhpalmieri:
See also #22787: do not build SageNB if
SAGE_PYTHON3=yes
. If this version fixes the Python 3 issues, then we should undo the change in #22787.
I propose we postpone dealing with this particular issue until Sage runs on python3.
comment:4 Changed 5 years ago by
I'd like to mention that after a first install of the new sagenb I sometimes have
sage -t --long --warn-long 54.6 local/lib/python2.7/site-packages/sagenb/notebook/worksheet.py ********************************************************************** File "local/lib/python2.7/site-packages/sagenb/notebook/worksheet.py", line 477, in sagenb.notebook.worksheet.Worksheet.__lt__ Failed example: W1 <= W2 Expected: True Got: False ********************************************************************** File "local/lib/python2.7/site-packages/sagenb/notebook/worksheet.py", line 479, in sagenb.notebook.worksheet.Worksheet.__lt__ Failed example: W2 <= W1 Expected: False Got: True
---something that I cannot reproduce after actually using sagenb. In fact even the re-run of the same test does not reproduce it. Not 100% sure how harmless these failures are. (I think they are harmless).
comment:5 Changed 5 years ago by
- Status changed from needs_review to needs_work
Test failures are never harmless.
comment:6 Changed 5 years ago by
If I reinstall the package then the test does not fail. Good luck debugging this, but I am not doing it.
As a workaround one can add running this test once in spkg-install and ignore its result.
comment:7 Changed 5 years ago by
- Status changed from needs_work to needs_info
Has anyone seen this before? I stopped paying much attention to sagenb in 2012, so I cannot recall anything.
comment:8 Changed 5 years ago by
hmm, comparison of worksheet has been changed recently (to avoid using cmp)
This lives in sagenb/notebook/worksheet.py
I think at least that maybe there lacks __ne__
and maybe also @total_ordering ?
comment:9 Changed 5 years ago by
I just made a pull request on github
comment:10 Changed 5 years ago by
- Commit changed from c29989b0113afe41a12bb5e73754734a41fc3be9 to 70fa3f8da73a00053655f9e03e0e92d333f1b988
comment:11 Changed 5 years ago by
- Status changed from needs_info to needs_review
OK, thanks to Frederic, the ordering of the worksheets is now fixed, and the doctests all pass too. (and the tarball is updated with these fixes, along with the checksum).
comment:12 Changed 5 years ago by
This works for me. I haven't tested that thoroughly, but all tests pass, tests do not create the "SAGE_ROOT/home" directory any more, and the notebook seems to work. "Live" help works, too. Anyone object to a positive review?
comment:13 follow-up: ↓ 14 Changed 5 years ago by
- Reviewers set to John Palmieri, Jeroen Demeyer
Let me just check 2 things:
- A rebuild from scratch (i.e. after
make distclean
)
- That the tarball is correctly produced.
comment:14 in reply to: ↑ 13 Changed 5 years ago by
- Status changed from needs_review to needs_work
Replying to jdemeyer:
Let me just check 2 things:
- A rebuild from scratch (i.e. after
make distclean
)
Doing this right now...
- That the tarball is correctly produced.
This is not looking good. Unless I'm missing something, the fixes mentioned in 11 are not in version 1.0 of SageNB. So the version number is misleading. Current SageNB master should be version 1.0.1 and then that tarball should be in Sage.
comment:15 Changed 5 years ago by
right, I shall bump up the version to 1.1.
comment:16 Changed 5 years ago by
If you want to use semantic versioning (not saying that you should), this is a bugfix, so it should be version 1.0.1
comment:17 Changed 5 years ago by
- Commit changed from 70fa3f8da73a00053655f9e03e0e92d333f1b988 to 3ce17147d5ac92c4f6fd87a27b9d4d880182fef9
comment:18 Changed 5 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
here is the bumped up version, repackaged so that the tarball is made by github.
comment:19 Changed 5 years ago by
- Status changed from needs_review to needs_work
A git repository is not a source distribution. I suggest to use the dist.sh
script to create the tarball.
That being said, it might be possible to install from a git repository, but then
- it should be clearly documented in
SPKG.txt
.
- the filename should be fixed (
1.0.1.tar.gz
is not acceptable because it doesn't containsagenb
).
comment:20 Changed 5 years ago by
- Commit changed from 3ce17147d5ac92c4f6fd87a27b9d4d880182fef9 to 7b85dc25f339d0988761c22da2f3dd4a2edc94c4
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
7b85dc2 | bumping up the version
|
comment:21 follow-up: ↓ 23 Changed 5 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
OK, I have followed your suggestion and used dist.sh
now.
PS. Regarding the somewhat unfortunate naming scheme for github "releases", I can only say that the tar.gz file gets renamed to the right one if you download it using a browser.
comment:22 Changed 5 years ago by
This also passes tests and works with my (cursory) usage after I built from scratch.
comment:23 in reply to: ↑ 21 Changed 5 years ago by
Replying to dimpase:
I can only say that the tar.gz file gets renamed to the right one if you download it using a browser.
My browser (GNU wget) didn't do that.
comment:24 Changed 5 years ago by
I'm testing this again...
comment:25 Changed 5 years ago by
- Status changed from needs_review to positive_review
comment:26 Changed 5 years ago by
- Branch changed from u/dimpase/sagenb10 to 7b85dc25f339d0988761c22da2f3dd4a2edc94c4
- Resolution set to fixed
- Status changed from positive_review to closed
please review!