Opened 5 years ago

Closed 5 years ago

#22821 closed defect (fixed)

Tweak Mac app handling of Jupyter upgrades

Reported by: kcrisman Owned by: iandrus
Priority: blocker Milestone: sage-8.0
Component: user interface Keywords:
Cc: iandrus, jdemeyer, vbraun Merged in:
Authors: Ivan Andrus Reviewers: Karl-Dieter Crisman, Volker Braun
Report Upstream: N/A Work issues:
Branch: 6ca6859 (Commits, GitHub, GitLab) Commit: 6ca685969e894679af93a2a489870ce1d5731dc7
Dependencies: Stopgaps:

Status badges

Description

Currently, the Mac app only checks once whether people want to upgrade sagenb to Jupyter - and the default save location is atrocious. From the author:

If it’s what I think you’re talking about, then yes it is Mac app specific.  It checks to see if there is a file at ~/.sage/sage_notebook.sagenb/users.pickle, and then asks if they want to "Upgrade", "Ask me Later", or "Don’task again".  It won’t ask again unless you choose "Ask me Later".  In particular, if you Upgrade then it won’t ask again (because presumably there is no need).  You can reset the check by running:

	defaults write org.sagemath.Sage askToUpgradeNB 1

The check itself is on line 511 of AppController.m in case you need to change anything.  I could easily add a menu option to upgrade, but I didn’t think it was worth it at the time.

Ideally we'd add a menu option as well.

Blocker because we will have a lot of people who only interact with this via the GUI who will be confused.

Change History (30)

comment:1 Changed 5 years ago by iandrus

  • Authors set to Ivan Andrus
  • Branch set to u/iandrus/mac-app-upgrade-nb
  • Commit set to b7ca77edd2e11f28582d8d41074f832a92bbd258
  • Owner changed from (none) to iandrus

There are two commits. The first unconditionally adds the menu item and the second greys it out if there are no files to be upgraded. It's fine with me if you want to just use the first because you think it will be less confusing.

I should note that I didn't test the actual upgrade.


New commits:

acf111atrac 22821 Add menu item for upgrading NB to Jupyter
b7ca77etrac 22821 Grey out the menu item if there is nothing to upgrade
Last edited 5 years ago by iandrus (previous) (diff)

comment:2 Changed 5 years ago by kcrisman

  • Cc vbraun added

From this sage-release thread:

I used the occasion of SD86 to try out: 

        sage-8.0.beta0-OSX_10.11.6-x86_64.app.dmg 

on a participant's mac. I dragged and dropped the app from the dmg 
into Applications. I get an icon in the tray. However clicking the 
icon just starts the legacy notebook without reference to Jupyter. 

Running Jupyter from the command line works fine: 

        /Applications/Sagexxx/sage -notebook jupyter 

Did I miss something along the way? 

Should we then have a different message if there is nothing to upgrade when people want to launch Sage? I believe the default is supposed to move to Jupyter at that point (Sage 8.0). Volker, do I understand that correctly that we'd want the Mac app to default to Jupyter at that time? So maybe we need a message asking about this.

Also, Ivan, I think a message warning people on upgrade to choose a good folder to save their upgrades might be good. I don't know how annoying that would be to implement. I do like the greyed-out option idea. Upgrades themselves work fine (or as fine as can be expected), I have tested this.

comment:3 Changed 5 years ago by git

  • Commit changed from b7ca77edd2e11f28582d8d41074f832a92bbd258 to aa00327db00dd4c9d034b84088822553e6c657a5

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

aa00327trac 22821 Prefer Jupyter by default

comment:4 follow-up: Changed 5 years ago by iandrus

I changed the default to prefer Jupyter (you can set it in Preferences). I could add a startup message to make them choose if we wanted.

There is already a setting for where Jupyter should be saved (~/Documents/Sage/? by default). Is the problem that the default is bad (or should be set at the very beginning), or that it's not being used? If the latter, is it not being used when starting the server or when exporting? Right now my checkout of Sage seems to be irrevocably broken (it always happens when I try to jump too many versions) so I can't test it right now.

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

Replying to iandrus:

I changed the default to prefer Jupyter (you can set it in Preferences). I could add a startup message to make them choose if we wanted.

Oh! I'll have to check that out - my sense was it was within the Sage app bundle directory but I may have just not been paying attention.

There is already a setting for where Jupyter should be saved (~/Documents/Sage/? by default). Is the problem that the default is bad (or should be set at the very beginning), or that it's not being used? If the latter, is it not being used when starting the server or when exporting? Right now my checkout of Sage seems to be irrevocably broken (it always happens when I try to jump too many versions) so I can't test it right now.

comment:6 Changed 5 years ago by kcrisman

I can't even get this to open Jupyter now. (I haven't added the commits on this ticket, to be clear, but I don't think that should impact it.) I open the app, it opens the sagenb, I sign out, stop the sagenb server, then try to start Jupyter. The menu icon turns green again, but the log shows no indication anything is going on, top shows no Python processes, and the check mark in the menu consistently is on "Stop Jupyter" no matter what I do.

Then when I click "Stop Jupyter" again I get the evil red menu icon, from which I cannot quit Sage. I have to quit from the Dock in that case. Ideas?

comment:7 Changed 5 years ago by vbraun

It doesn't work for me either, always starts sagenb.

  • To run legacy SageNB: sage --notebook=sagenb
  • To run Jupyter: sage --notebook=jupyter
  • To run the SageNB -> Jupyter converter web app: sage --notebook=export

The menu points are

  • Start SageNB Server
  • Stop SageNB Server
  • Start Jupyter <-- does nothing? Log is empty
  • Stop Jupyter
  • Start SageNB Server <-- again
  • Stop SageNB Server <-- again

Btw preferences -> Use alternate Sage executable is not saved (always unset after restart) and apparently never used

comment:8 Changed 5 years ago by iandrus

I pushed a fix to ensure that the Jupyter directory exists (which was why it wouldn't start).

However, Use Alternate Sage Executable has worked great for me for a long time. Moreover, you should only see Start/Stop? SageNB Server in the menu once. One set should be hidden based on the setting of preferSageNB. Volker, can you tell me more about what problems you are experiencing?

comment:9 Changed 5 years ago by git

  • Commit changed from aa00327db00dd4c9d034b84088822553e6c657a5 to a06cabdacb9310c8cd5ebf0e187363f55d4e5840

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

a06cabdtrac 22821 Create Jupyter directory before use and prompt if it doesn’t exist

comment:10 Changed 5 years ago by kcrisman

Awesome, yes, I definitely didn't have that directory. When I change the default in preferences this works. However for some reason the check mark does not move away from "Stop Jupyter" no matter what I do.

When I switch (still without this ticket) to not preferring old sagenb, I don't see gray out or hidden. It does switch to the "lower" set of start/stop.

I think if you can change the "please wait" page to note that they can find the url (including token) if the auto-open doesn't work in the "View log", that would be a very helpful easy change. Also, that loading page should mention where they can switch that preference (in the Preferences -> Notebook and also to start/stop in menu icon). I hope that is not too much trouble.

I forget by now how to test an update to just the Mac app - I really don't want to have to go to all the trouble to build a new binary, especially now that the binary-pkg is an even bigger behemoth (or seems so to me). Anyone remember? Pull and make, but then what?

comment:11 Changed 5 years ago by git

  • Commit changed from a06cabdacb9310c8cd5ebf0e187363f55d4e5840 to 5aabb312cfa68e73d6edd699f3fd79cc3ec65033

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

ca3f558trac 22821 don’t check the "Stop Sage" menu item
5aabb31trac 22821 Open loading page for Jupyter as well as SageNB

comment:12 Changed 5 years ago by iandrus

It still shows the possibility of starting either server. My thought was that even if you prefer one, you should have the option of using the other.

It looks like the checkmark of "Stop Jupyter" was just set for some reason. I wonder when I messed that up. I have fixed it.

I changed the loading page to work for Jupyter too. Hopefully, it's something like what you had in mind.

To test, just the mac app

open ./src/mac-app/Sage.xcodeproj/ # will open in Xcode

Then Command-R will build and run Sage.app (you'll have to set it to Use Alternate Sage Exectable in Preferences). I think the only thing that doesn't work is the that Sage Version in About Sage isn't set (but who cares?).

comment:13 Changed 5 years ago by kcrisman

Thanks for the testing instructions! It gave me all kinds of warnings but I ignored them because I think they are to stay backwards-compatible - good.

Unfortunately, it seems to still be starting the sagenb? I don't have that preference set right now, yet it is still starting sagenb by default (even when I just do "Open Notebook" from the menu items). Is there something else that has to happen to make that start?

Perhaps more importantly, I can't find the upgrade menu item. I know I am using the more recent version because the check mark is gone and the loading page is updated. Where should I look?

In general though I can't say enough about your commitment to this project and how nice and usable it is. We get many people on ask.sagemath who know nothing other than the app, which I think is a testament to this labor of love. Someday I'll learn Obj-C and be able to help ...

comment:14 Changed 5 years ago by iandrus

I put the upgrade menu item in the main menu, but I forgot to put it in the status bar item (which I assume is what you're using). I pushed a fix.

The point about Open Notebook and New Worksheet is interesting. Those actually only work with SageNB because there was no way to get them to work with Jupyter when I tried a few years ago. I don't remember the details, but I think it had to do with authentication.

Maybe. Perhaps we should hide/disable them entirely if SageNB isn't the preferred notebook. Alternately, we could rename them to indicate they are SageNB only, but I think that's a bit clunky.

I'm glad you like the app, but I have to say it wouldn't be nearly as good without you testing it and keeping me honest. :-)

Version 0, edited 5 years ago by iandrus (next)

comment:15 Changed 5 years ago by kcrisman

You may have pushed a fix locally, but maybe it's not pushed to Trac?

I think that if "Open Notebook" became an alias to "Start X" where X is the default in the Preferences, that would solve that. Then "New Worksheet" could either go away (in both location, menu and icon) or could be renamed. My first instinct is to say go away, though I'm not sure why. It is certainly useful.

By the way, if the loading page doesn't already mention the preference for setting your preferred notebook, that would be useful. I know it's getting a bit busy.

comment:16 Changed 5 years ago by iandrus

You're right. I tried to push it and it failed. I'm still not sure why. I can't push it now either, maybe because I'm at work?

I'll try to look into the others tonight.

comment:17 Changed 5 years ago by git

  • Commit changed from 5aabb312cfa68e73d6edd699f3fd79cc3ec65033 to 84fb0088b171dcd9236366265d896b349daad3f8

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

84fb008Add Upgrade SageNB menu to the status bar item.

comment:18 Changed 5 years ago by git

  • Commit changed from 84fb0088b171dcd9236366265d896b349daad3f8 to 1cfce71a794ca7812873683cc1e8473c984443f4

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

b52cfeatrac 22821 Add menu item for upgrading NB to Jupyter
63f7e83trac 22821 Prefer Jupyter by default
dd241a7trac 22821 Create Jupyter directory before use and prompt if it doesn’t exist
7e734a5trac 22821 Open loading page for Jupyter as well as SageNB
1cfce71trac 22821 Clean up menus a little

comment:19 Changed 5 years ago by iandrus

I made Open Notebook into "open a currently running server or start the preferred," and hid New Worksheet unless they prefer SageNB.

Since I think we're getting close to something usable (if not there already) I rebased on develop and cleaned up the commits a little. I hope that's not too onerous.

comment:20 Changed 5 years ago by kcrisman

Perfect - and, assuming I can do one final run-through of looking at this in the next few days, I would hope it's better to merge in Sage 8.0 than keep working at this. Thank you!

comment:21 Changed 5 years ago by iandrus

  • Status changed from new to needs_review

comment:22 Changed 5 years ago by kcrisman

Sorry about the delay - I am building the latest Sage beta and will do a final check on this, but I have no reason to believe it's not good enough to go.

comment:23 Changed 5 years ago by kcrisman

This looks good. I do note that the new push seems to still default to the legacy notebook even when I didn't have that clicked. Is that a glitch on my computer because I tested this? It was already not checked when I started the app on this most recent one.

Most importantly, the upgrade is indeed now always accessible - by far the most important part.

So the only issue is that on start only it seems to always do legacy sagenb even if not checked to do so. My guess is that there is still some default that went back to "yes" or something.

I made Open Notebook into "open a currently running server or start the preferred," and hid New Worksheet unless they prefer SageNB.

Yes, this works now. I'll note that "New Worksheet" always appears in the File menu (as opposed to the menu item). I don't know if that is intended, though.

Otherwise this should be merged and people should enjoy Sage 8.0!

comment:24 Changed 5 years ago by kcrisman

So the only issue is that on start only it seems to always do legacy sagenb even if not checked to do so. My guess is that there is still some default that went back to "yes" or something.

It is amusing to unset the default Jupyter directory, close Sage, restart it, be asked to pick a directory (which is awesome), and then have the sagenb notebook start up ;-)

comment:25 Changed 5 years ago by kcrisman

  • Status changed from needs_review to needs_work
  • Work issues set to fix default start

comment:26 Changed 5 years ago by kcrisman

Ivan, is the problem here in AppController.m?

-(IBAction)openNotebook:(id)sender{
    if ( jupyterURL != nil ) {
        [self browseRemoteURL:jupyterURL];
    } else if ( port != 0 || (defaults && [defaults boolForKey:@"preferSageNB"]) ) {
        [self browseLocalSageURL:@""];
    } else {
        [self startJupyter:sender];
    }
}

Or conceivably in start-sage.sh

if [ "$NB_TYPE" = jupyter ]; then

./sage --notebook=jupyter $4 >> "$SAGE_LOG" 2>> "$SAGE_LOG" &

Maybe this hasn't actually been used properly?

I couldn't find any other place where this seems to be done. In particular

   } else if ( port != 0 || (defaults && [defaults boolForKey:@"preferSageNB"]) ) {

maybe somehow port is set (I couldn't figure out how) accidentally when we start off?

Volker, truly this is otherwise ready to go, just FYI.

comment:27 Changed 5 years ago by git

  • Commit changed from 1cfce71a794ca7812873683cc1e8473c984443f4 to 6ca685969e894679af93a2a489870ce1d5731dc7

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

6f89dd5trac 22821 Hide New Worksheet in the File menu too
6ca6859trac 22821 Correctly start Jupyter on startup based on setting

comment:28 Changed 5 years ago by iandrus

  • Status changed from needs_work to needs_review

Okay, those two should be fixed. Thanks so much for your patience Karl-Dieter.

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

comment:29 Changed 5 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman, Volker Braun
  • Status changed from needs_review to positive_review
  • Work issues fix default start deleted

Yes!!!

Only tiny remaining thing I could find was the preference "Start server when opening sws files" might accidentally start Jupyter if that pref is set, but apparently even that works right. Yay! Thank you so much.

comment:30 Changed 5 years ago by vbraun

  • Branch changed from u/iandrus/mac-app-upgrade-nb to 6ca685969e894679af93a2a489870ce1d5731dc7
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.