Opened 7 years ago

Closed 5 years ago

#14840 closed defect (fixed)

SageNB package contains many packages

Reported by: felixs Owned by: felixs
Priority: major Milestone: sage-7.2
Component: packages: standard Keywords: notebook, days77
Cc: jondo, kcrisman Merged in:
Authors: Jeroen Demeyer Reviewers: Salvatore Stella, Dima Pasechnik
Report Upstream: Completely fixed; Fix reported upstream Work issues:
Branch: b5fb38b (Commits) Commit: b5fb38b63c8c654259f1df110c31ed153be2b8f3
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

This ticket changes SageNB from a distribution to a normal Python package. This means unbundling everything which used to be part of the SageNB distribution.

The package webassets is simply removed, since it doesn't seem to be used by anything in Sage or SageNB. This thread indicates that webassets is used by the newui branch of SageNB but not in vanilla SageNB. So it has no reason being in Sage currently.

New upstream tarballs:

Change History (78)

comment:1 Changed 7 years ago by jondo

  • Cc jondo added

comment:2 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:3 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:4 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:5 Changed 6 years ago by vbraun

  • Priority changed from critical to major

comment:6 Changed 5 years ago by jdemeyer

  • Authors changed from Felix Salfelder to Jeroen Demeyer
  • Component changed from notebook to packages: standard
  • Keywords toplevel build system removed
  • Milestone changed from sage-6.4 to sage-7.2

comment:7 Changed 5 years ago by jdemeyer

  • Cc kcrisman added

comment:8 Changed 5 years ago by jdemeyer

  • Summary changed from notebook package contains many packages to sagenb package contains many packages

comment:9 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:10 Changed 5 years ago by jdemeyer

  • Keywords days77 added

comment:11 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:12 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:13 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:14 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:15 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:16 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:17 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:18 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:19 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:20 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:21 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:22 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/14840

comment:23 Changed 5 years ago by git

  • Commit set to 14afa817cb1e0a56b690041297f730c64b674192

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

14afa81Split up the sagenb distribution

comment:24 Changed 5 years ago by git

  • Commit changed from 14afa817cb1e0a56b690041297f730c64b674192 to 34e7693aaeabfeb4fc4477d5a80b724f764dd37f

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

34e7693Split up the sagenb distribution

comment:25 Changed 5 years ago by git

  • Commit changed from 34e7693aaeabfeb4fc4477d5a80b724f764dd37f to 161ac3ba998689b9a3a3435ed85e4316cd61fb77

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

161ac3bSplit up the sagenb distribution

comment:26 Changed 5 years ago by kcrisman

  • Description modified (diff)

This is really good, Jeroen, though I'm surprised you have time for it, I always thought this was lower priority :) See also #17306, would there need to be any followup to remove whatever was done there? See also https://github.com/sagemath/sagenb/issues/258 which could hopefully be dealt with at this time too.

comment:27 follow-up: Changed 5 years ago by jdemeyer

I'm just removing mathjax from the sagenb package. I don't think that more changes are needed regarding mathjax in sagenb.

comment:28 Changed 5 years ago by jdemeyer

  • Description modified (diff)
  • Report Upstream changed from N/A to Reported upstream. No feedback yet.

comment:29 Changed 5 years ago by jdemeyer

  • Status changed from new to needs_review

comment:30 Changed 5 years ago by jdemeyer

  • Description modified (diff)
  • Summary changed from sagenb package contains many packages to SageNB package contains many packages

comment:31 follow-up: Changed 5 years ago by etn40ff

Looks good to me.

The only minor doubt I have is about using a non released version of sagenb. This being a stopgap solution I do not see it as being an issue though.

comment:32 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:33 in reply to: ↑ 31 ; follow-up: Changed 5 years ago by jdemeyer

Replying to etn40ff:

The only minor doubt I have is about using a non released version of sagenb.

Let's hope that upstream sagenb can accept it.

comment:34 in reply to: ↑ 33 Changed 5 years ago by kcrisman

The only minor doubt I have is about using a non released version of sagenb.

Let's hope that upstream sagenb can accept it.

That shouldn't be a problem as long as we can verify that installation and use of sagenb is still smooth with this - indeed, it would simplify things rather substantially for upstream sagenb.

comment:35 follow-up: Changed 5 years ago by jdemeyer

So, @kcrisman, would you give positive review?

comment:36 in reply to: ↑ 35 Changed 5 years ago by kcrisman

So, @kcrisman, would you give positive review?

Yes when I can try it - or someone else can try it and make sure everything works okay.

comment:37 Changed 5 years ago by jdemeyer

  • Reviewers set to Salvatore Stella

comment:38 follow-up: Changed 5 years ago by etn40ff

I did few testing and it looks like it should be ok for me. Shall we wait for @kcrisman to give it a spin?

comment:39 in reply to: ↑ 27 Changed 5 years ago by kcrisman

I'm just removing mathjax from the sagenb package. I don't think that more changes are needed regarding mathjax in sagenb.

I found what I was looking for, awesome:

-# the following line can be removed once sagenb does not ship mathjax anymore.
-rm -rf mathjax
Last edited 5 years ago by kcrisman (previous) (diff)

comment:40 in reply to: ↑ 38 ; follow-up: Changed 5 years ago by kcrisman

I did few testing and it looks like it should be ok for me. Shall we wait for @kcrisman to give it a spin?

That won't happen for some time (e.g. not until at least after the weekend). I think what is most needed here is for someone to try building Sage from scratch using this and making sure everything does indeed get put in the right places, and then that all the usual sagenb functionality (jsmol, translations, etc.) still work. I have no doubt that it will work but I just don't have the time to do it; if Salvatore (is that etn40ff?) can vouch for this then it's good.

Only one thing; if there is then an "official" sagenb 0.12 will that complicate matters later, would it have to be 0.12.1 for everything to build properly?

comment:41 Changed 5 years ago by etn40ff

I did recompile sage from scratch and run complete doctest on it. Stupidly I did not run the notebook tough: I got distracted then forgot.

Reasonably I will not be able to recompile again before the end of the weekend either but I will gladly take care of this on Monday if nobody beats me to it.

(yes, I am Salvatore, sorry for the weird username)

comment:42 in reply to: ↑ 40 ; follow-ups: Changed 5 years ago by jdemeyer

Replying to kcrisman:

Only one thing; if there is then an "official" sagenb 0.12

Is there actually a place where the "official" sagenb tarballs are hosted? If not, then we can just define my tarball as the official one.

comment:43 in reply to: ↑ 42 ; follow-up: Changed 5 years ago by kcrisman

Only one thing; if there is then an "official" sagenb 0.12

Is there actually a place where the "official" sagenb tarballs are hosted? If not, then we can just define my tarball as the official one.

Oh, I don't think I saw that this was 0.12 even in this commit. How would I (eventually) merge that in the github master branch without creating the extra merge commit?

comment:44 in reply to: ↑ 43 ; follow-up: Changed 5 years ago by jdemeyer

Replying to kcrisman:

Oh, I don't think I saw that this was 0.12 even in this commit. How would I (eventually) merge that in the github master branch without creating the extra merge commit?

I don't have enough experience managing a github project to answer this using github. But it should be possible to do the merge using the standard git merge command line.

comment:45 follow-up: Changed 5 years ago by dimpase

I'd rather see all this becoming a bunch of optional packages, rather than standard packages.

comment:46 Changed 5 years ago by kcrisman

But it should be possible to do the merge using the standard git merge command line.

True.

I'd rather see all this becoming a bunch of optional packages, rather than standard packages.

Is that possible without making sagenb optional, though? Otherwise we might as well stick with the previous situation.

comment:47 in reply to: ↑ 45 Changed 5 years ago by jdemeyer

Replying to dimpase:

I'd rather see all this becoming a bunch of optional packages, rather than standard packages.

But sagenb itself is still standard, so I don't see the point of marking those other packages optional. In any case, this ticket here will make it possible to make sagenb and its dependencies optional in the future.

comment:48 follow-up: Changed 5 years ago by dimpase

I was suggesting to make all of sagenb optional. Surely it's not used too much nowadays, as many people would use jupyther notebook instead. (I am seldom using any of these, I don't know how small such a minority is).

comment:49 in reply to: ↑ 44 Changed 5 years ago by dimpase

Replying to jdemeyer:

Replying to kcrisman:

Oh, I don't think I saw that this was 0.12 even in this commit. How would I (eventually) merge that in the github master branch without creating the extra merge commit?

turn merge squashing on, then merge as you'd normally merge a PR on github. https://help.github.com/articles/about-pull-request-merge-squashing/

I don't have enough experience managing a github project to answer this using github. But it should be possible to do the merge using the standard git merge command line.

With subsequent git rebase?

comment:50 in reply to: ↑ 48 ; follow-up: Changed 5 years ago by jdemeyer

Replying to dimpase:

I was suggesting to make all of sagenb optional.

That's certainly not a decision to be taken on this ticket. SageNB is still the default notebook in Sage.

comment:51 in reply to: ↑ 50 Changed 5 years ago by kcrisman

I was suggesting to make all of sagenb optional.

That's certainly not a decision to be taken on this ticket. SageNB is still the default notebook in Sage.

Given the support requests that still come in, and the fact that sage-devel is (like any other developer list) notoriously populated by early adopters, I'd say it's unlikely we should make sagenb optional. Early in my work with Sage I had so many questions from people who wanted to use it but we didn't have an automatic Maple or Mma translator, so they were not interested - sunk effort. I'm pretty sure there is a sizable (not on the order of Maple or Mma users, of course) contingent of people with the same sunk effort in sagenb. I do plan to look at the converter to ipynb ticket this summer, since I see the way things are going, but removing sagenb as standard makes no sense, because the people most likely to use it are the people least likely to have the infrastructure to add it back in (Windows and Mac binary users). (A "personal" version of SMC would not be amiss in that discussion either, so people don't always have to be online to use their Sage, but that isn't for this ticket.)

comment:52 in reply to: ↑ 42 Changed 5 years ago by jdemeyer

Regarding the comment

Replying to kcrisman:

Only one thing; if there is then an "official" sagenb 0.12 will that complicate matters later, would it have to be 0.12.1 for everything to build properly?

can you please answer this important question:

Replying to jdemeyer:

Is there actually a place where the "official" sagenb tarballs are hosted? If not, then we can just define my tarball as the official one.

My impression was that, whenever you made a new SageNB release, you just host it somewhere on a personal website. So then I don't see why my sagenb-0.12 on this ticket would be less official than your previous sagenb releases.

But please tell me if I'm wrong...

comment:53 follow-up: Changed 5 years ago by dimpase

well, the upstream lives on github. I suggest we merge the corresponding pull requests there, before creating a release, so that the tarball does come from the "official" repo.

comment:54 in reply to: ↑ 53 ; follow-up: Changed 5 years ago by jdemeyer

Replying to dimpase:

well, the upstream lives on github. I suggest we merge the corresponding pull requests there, before creating a release, so that the tarball does come from the "official" repo.

If kcrisman merges just this one pull request and nothing else, then my tarball and the "official" will have exactly the same sources (they might even be the same git commit if a fast-forward merge is done), so they are effectively indistinguishable.

comment:55 in reply to: ↑ 54 ; follow-up: Changed 5 years ago by kcrisman

well, the upstream lives on github. I suggest we merge the corresponding pull requests there, before creating a release, so that the tarball does come from the "official" repo.

If kcrisman merges just this one pull request and nothing else, then my tarball and the "official" will have exactly the same sources (they might even be the same git commit if a fast-forward merge is done), so they are effectively indistinguishable.

Yeah, as long as I don't have to create an extra merge commit (which is not possible on GH but should be possible command line, as you say) then that is fine.

In fact, without the weird packaging thing, the "official" tarball would indeed be the ones on Github now; the reason they aren't currently is because of the subpackage stuff, I believe. I'm only concerned about the repos looking the same, not where some "official" package lives.

I'm sorry I can't do anything "officially" on GH now regarding this (I mean merging, because then I feel responsible and have to do testing etc.). But this solution is fine with me, and I am one of the few people who cares about this, so no worries.

comment:56 in reply to: ↑ 55 ; follow-up: Changed 5 years ago by dimpase

Replying to kcrisman:

well, the upstream lives on github. I suggest we merge the corresponding pull requests there, before creating a release, so that the tarball does come from the "official" repo.

If kcrisman merges just this one pull request and nothing else, then my tarball and the "official" will have exactly the same sources (they might even be the same git commit if a fast-forward merge is done), so they are effectively indistinguishable.

Yeah, as long as I don't have to create an extra merge commit (which is not possible on GH

why? did you look at the link I posted? https://help.github.com/articles/about-pull-request-merge-squashing/

but should be possible command line, as you say) then that is fine.

In fact, without the weird packaging thing, the "official" tarball would indeed be the ones on Github now; the reason they aren't currently is because of the subpackage stuff, I believe. I'm only concerned about the repos looking the same, not where some "official" package lives.

I'm sorry I can't do anything "officially" on GH now regarding this (I mean merging, because then I feel responsible and have to do testing etc.).

I presume one just has to look that it builds and runs. It's merely re-packaging of the same thing, right?

But this solution is fine with me, and I am one of the few people who cares about this, so no worries.

comment:57 in reply to: ↑ 56 Changed 5 years ago by kcrisman

Yeah, as long as I don't have to create an extra merge commit (which is not possible on GH

why? did you look at the link I posted? https://help.github.com/articles/about-pull-request-merge-squashing/

Sorry, no I didn't see that update to this ticket, my apologies (surely not the only thing I've missed). I don't see a way to add commits without a merge and without squashing, maybe that's if one unchecks both buttons?

I presume one just has to look that it builds and runs. It's merely re-packaging of the same thing, right?

Well, I tend to want to slowly confirm all that myself, check weird edge cases, etc. Which is perhaps unnecessary and my downfall, but anyway it's as it is. The situation here can be back-read into sagenb one way or another once my semester ends. I can't do more right now.

comment:58 Changed 5 years ago by kcrisman

PS with respect to the "modularization" sagenb thread, the sooner sagenb gets back in Sage proper, the better! Also, I am indeed interested in maintaining it, but just have very little time for it. I'm sorry about that, but that's just how it is. :-(

comment:59 Changed 5 years ago by dimpase

IMHO sagenb should not drag all of its dependencies into sage. Why can't the installation script simply run sage --pip?

comment:60 follow-up: Changed 5 years ago by jhpalmieri

Sage policy is to not require internet access to build Sage: you should be able to download a complete tarball and build from that. So we have to include tarballs for all of these dependencies, at which point why not make them Sage packages? Then we know they are available if they are needed elsewhere in Sage, and we can make any other dependencies explicit.

comment:61 in reply to: ↑ 60 Changed 5 years ago by dimpase

Replying to jhpalmieri:

Sage policy is to not require internet access to build Sage: you should be able to download a complete tarball and build from that.

perhaps it's time for it to change; after all we tell users to install ssl support via pip, creating a lot of pain and confusion along the way, as we don't package the corresponding script thanks to exactly this prohibition. If we changed this policy we would only improve the nb-related procedures here.

So we have to include tarballs for all of these dependencies, at which point why not make them Sage packages? Then we know they are available if they are needed elsewhere in Sage, and we can make any other dependencies explicit.

well, in this case they are not needed anywhere else in sage.

Last edited 5 years ago by dimpase (previous) (diff)

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

I tend to agree that we should just exclude web-based UIs (SageNB + Jupyter) from the "self-contained tarball" and make them pip-type optional packages instead. If you don't have internet access then how come you have a browser installed? Also the whole OpenSSL can of worms if you really want to run a server.

But this should be changed at a major release, not now.

comment:63 in reply to: ↑ 62 ; follow-ups: Changed 5 years ago by jhpalmieri

Replying to vbraun:

I tend to agree that we should just exclude web-based UIs (SageNB + Jupyter) from the "self-contained tarball" and make them pip-type optional packages instead. If you don't have internet access then how come you have a browser installed?

Because you live in a third world country and have inconsistent access? Because you're traveling and want to build Sage while on an airplane (and without paying the wifi fees)? You can come up with plenty of reasons.

comment:64 in reply to: ↑ 63 Changed 5 years ago by dimpase

Replying to jhpalmieri:

Replying to vbraun:

I tend to agree that we should just exclude web-based UIs (SageNB + Jupyter) from the "self-contained tarball" and make them pip-type optional packages instead. If you don't have internet access then how come you have a browser installed?

Because you live in a third world country and have inconsistent access? Because you're traveling and want to build Sage while on an airplane (and without paying the wifi fees)? You can come up with plenty of reasons.

Inconsistent access is a nightmare if you need to get a large file (e.g. current Sage source tarball), as opposed to several small files.

Besides, mind you, much more popular than Sage software distribution systems, e.g. ones for Python, Haskell, OCaml do require internet access, by default. Convenience for the majority (and for developers) ought to come before exotic usage cases.

comment:65 in reply to: ↑ 63 ; follow-up: Changed 5 years ago by vbraun

IMHO those use cases are better served by having a script that downloads all standard+optional packages. I already have a script where you just have to

sage-package download :standard: :optional:

to grab them all in one go.

comment:66 in reply to: ↑ 65 Changed 5 years ago by jhpalmieri

Replying to dimpase:

Replying to jhpalmieri:

Replying to vbraun:

I tend to agree that we should just exclude web-based UIs (SageNB + Jupyter) from the "self-contained tarball" and make them pip-type optional packages instead. If you don't have internet access then how come you have a browser installed?

Because you live in a third world country and have inconsistent access? Because you're traveling and want to build Sage while on an airplane (and without paying the wifi fees)? You can come up with plenty of reasons.

Inconsistent access is a nightmare if you need to get a large file (e.g. current Sage source tarball), as opposed to several small files.

Hence tarballs on thumb drives.

Besides, mind you, much more popular than Sage software distribution systems, e.g. ones for Python, Haskell, OCaml do require internet access, by default. Convenience for the majority (and for developers) ought to come before exotic usage cases.

If/when we make sagenb into an optional or experimental package, I have no objections to doing the same with these dependencies. But until then, in what way is including a dozen packages an inconvenience? It doesn't take up any extra disk space or extra time building Sage since we're already including them, it just adds a few directories in build/pkgs. How is this more of an inconvenience than requiring internet access during compilation of Sage?

Replying to vbraun:

IMHO those use cases are better served by having a script that downloads all standard+optional packages. I already have a script where you just have to

sage-package download :standard: :optional:

to grab them all in one go.

And how are Sage developers supposed to know about this, if they are going to be traveling and without good internet access for a while? It sounds like a good idea, but I still don't see what it saves vs. making these standard Sage packages (for as long as sagenb is a standard Sage package, but no longer).

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

As I said above, I do agree that we should make the packages standard now. But in the future we should probably reevaluate the wisdom for why we are making source tarballs with two separate web-based notebooks.

comment:68 in reply to: ↑ 67 Changed 5 years ago by kcrisman

As I said above, I do agree that we should make the packages standard now. But in the future we should probably reevaluate the wisdom for why we are making source tarballs with two separate web-based notebooks.

I'm not quite sure why the "web-based" is relevant here; why not say "browser-based" instead? I use my browser all the time without being on the internet - primarily as a Sage GUI, but most browsers now save things for later reading, I often save pages to show to students in situations where wireless isn't working and it opens in the browser, etc. After all, most people aren't running their local sagenb or Jupyter as a web service.

Anyway, this solution seems to me like a good intermediate solution.

comment:69 Changed 5 years ago by dimpase

OK, I'll be happy to review this; I just merged your pull request. Could you perhaps make an official tarball, based off https://github.com/sagemath/sagenb ?

comment:70 Changed 5 years ago by jdemeyer

  • Description modified (diff)

No need for a new tarball, just define the one already on this ticket as the official one.

comment:71 Changed 5 years ago by git

  • Commit changed from 161ac3ba998689b9a3a3435ed85e4316cd61fb77 to b5fb38b63c8c654259f1df110c31ed153be2b8f3

Branch pushed to git repo; I updated commit sha1. New commits:

b5fb38bUse "setup.py install" to install Python packages

comment:72 follow-up: Changed 5 years ago by jdemeyer

Volker just reminded me that we cannot use pip install . currently, since that needs SSL.

comment:73 in reply to: ↑ 72 ; follow-up: Changed 5 years ago by dimpase

Replying to jdemeyer:

Volker just reminded me that we cannot use pip install . currently, since that needs SSL.

pip is a standard Sage package. Are you saying that it is broken, generally speaking? Probably if you build everything without ssl then it would work too, no?

comment:74 in reply to: ↑ 73 Changed 5 years ago by jdemeyer

Replying to dimpase:

Replying to jdemeyer:

Volker just reminded me that we cannot use pip install . currently, since that needs SSL.

pip is a standard Sage package. Are you saying that it is broken, generally speaking?

Sort of: it always requires SSL even if you don't make any internet connection.

Probably if you build everything without ssl then it would work too, no?

Apparently not.

comment:75 Changed 5 years ago by dimpase

  • Report Upstream changed from Reported upstream. No feedback yet. to None of the above - read trac for reasoning.
  • Reviewers changed from Salvatore Stella to Salvatore Stella, Dima Pasechnik

looks OK on linux. let me see if it's also OK on OSX, just in case.

comment:76 Changed 5 years ago by dimpase

  • Status changed from needs_review to positive_review

OK.lightly tested on few random worksheets, old and new, on OSX too.

comment:77 Changed 5 years ago by jdemeyer

  • Report Upstream changed from None of the above - read trac for reasoning. to Completely fixed; Fix reported upstream

Thanks!

comment:78 Changed 5 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/14840 to b5fb38b63c8c654259f1df110c31ed153be2b8f3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.