sympy / sympy.github.com

SymPy's web page (sympy.org)
https://sympy.org/
188 stars 107 forks source link

JupyterLite-based shell at https://www.sympy.org/en/shell.html #174

Closed ivanistheone closed 2 years ago

ivanistheone commented 2 years ago

This is a derivative PR of the original work by @jptio PR#169.

certik commented 2 years ago

@ivanistheone thanks! Please ping me once this is ready.

ivanistheone commented 2 years ago

I added support for ?evaluate= urls. Here are some test URLs to try:

Still working on webcopy in sidebar, will take of WIP when that is done.

ivanistheone commented 2 years ago

@jtpio I ran into an interesting edge case when opening the console in a background tab, jupyterlite seems to think there is no Kernel:

Screen Shot 2022-04-19 at 12 23 10 PM

To reproduce:

Question 1: I was wondering if you might have seen this before and if there is a workaround to avoid this.

ivanistheone commented 2 years ago

Hi @jtpio I was looking at the js console while the JupyterLite shell is loading ...

loading 1 loading 2 loading 3 loading 4 loading 5

... and I have another question for you:

Question 2: Is there a way to customize the loading process to skip certain packages (e.g. skip matplotlib)?

Currently it takes 22 seconds to load the SymPy shell on my computer (desktop, medium-fast internet connection), so I was wondering if there might be a way to shave a few seconds off of that by customizing the code in jupyterlite/demo. (Not for this PR but for followup perf. optimization work).

asmeurer commented 2 years ago

We don't need to block this on anything that can be fixed upstream in jupyterlite, because we can fix those things upstream after this is merged and the site will automatically get the improvements.

jtpio commented 2 years ago

Question 1: I was wondering if you might have seen this before and if there is a workaround to avoid this.

I might have seen it once before. Looks like this should be reproducible with https://jupyterlite.github.io/demo/repl/ or https://jupyterlite.rtfd.io/en/latest/try/repl?

Looks like a race condition with the kernels not having the time to register themselves before the application starts. Would you mind opening an issue in https://github.com/jupyterlite/jupyterlite ?

Question 2: Is there a way to customize the loading process to skip certain packages (e.g. skip matplotlib)?

Yes there was an attempt at delaying this import in the past but it caused some issues with the recursion limit in Pyodide. This is currently tracked in https://github.com/jupyterlite/jupyterlite/issues/254.

jtpio commented 2 years ago

Looks like a race condition with the kernels not having the time to register themselves before the application starts. Would you mind opening an issue in https://github.com/jupyterlite/jupyterlite ?

Opened https://github.com/jupyterlite/jupyterlite/issues/610 to track it upstream.

jtpio commented 2 years ago

Opened jupyterlite/jupyterlite#610 to track it upstream.

@ivanistheone This should now be fixed in 0.1.0b6: https://github.com/jupyterlite/jupyterlite/releases/tag/v0.1.0b6

With the demo repo updated to use the latest release: https://jupyterlite.github.io/demo/repl/?toolbar=1&kernel=python

asmeurer commented 2 years ago

What's the status of this? I'd love to have SymPy Live phased out before we do the next SymPy release since the next SymPy release will also completely remove it from the documentation. At that point, we can shut down the App Engine server, and add a note to the repo that if someone still wants to run it, they should deploy it to the App Engine themselves.

ivanistheone commented 2 years ago

What's the status of this?

Yeah sorry for delay. I just have to fix the webcopy then we can merge this, which will conclude PHASE 1. I'll work on this tomorrow and update the demo link for review.

I'd love to have SymPy Live phased out before we do the next SymPy release since the next SymPy release will also completely remove it from the documentation. At that point, we can shut down the App Engine server, and add a note to the repo that if someone still wants to run it, they should deploy it to the App Engine themselves.

As soon as we ship this PR, I'll work on PHASE 3 and can provide and time estimate then. @asmeurer @certik Is there a target date in mind for the next release?

Since the new docs have removed the shell component, live.sympy.org is an independent service so could be [deployed](What's the status of this?

Yeah sorry for delay. I just have to fix the webcopy and we can merge this, which will conclude PHASE 1.

I'd love to have SymPy Live phased out before we do the next SymPy release since the next SymPy release will also completely remove it from the documentation. At that point, we can shut down the App Engine server, and add a note to the repo that if someone still wants to run it, they should deploy it to the App Engine themselves.

As soon as we ship this PR, I'll work on PHASE 3 and can provide and time estimate then. @asmeurer @certik Is there a target date in mind for the next release?

Since the new docs have removed the shell component, live.sympy.org is an independent service so could be launched independently of the next SymPy release.

I'm thiniking at least a few weeks using the (upcoming) http://sympy.org/en/shell.html for testing to make sure it works.

if bool(PHASE2) then PHASE4().) independently of the next SymPy release.

I'm thiniking at least a few weeks using the (upcoming) http://sympy.org/en/shell.html for testing to make sure it works. In other words, if bool(PHASE2) then PHASE4().

ivanistheone commented 2 years ago

@jtpio I've been doing some testing on the demo link from various browsers and it works on most of them (on an old iPhone).

Only one that didn't work was Safari (on iOS medium old). Is there something about Safari on iOS that doesn't allow wasm?

doesntloadOniOS

asmeurer commented 2 years ago

I didn't know about that Google Doc with the phase plan. The plan looks good to me.

As soon as we ship this PR, I'll work on PHASE 3 and can provide and time estimate then. @asmeurer @certik Is there a target date in mind for the next release?

The person to ask here is @oscarbenjamin.

The SymPy release and SymPy Live updates don't necessarily have to happen in tandem, but it's worth noting that the next release of SymPy will remove it from the docs, so we can't shut down the AppEngine at least until then.

Also, how hard would it be to add some text to the AppEngine version that it is being deprecated and redirecting people to the new version? That way there will be some warning for people before it is shut down completely.

jtpio commented 2 years ago

Thanks @ivanistheone!

Only one that didn't work was Safari (on iOS medium old). Is there something about Safari on iOS that doesn't allow wasm?

Were you able to get some information from the dev tools console? Curious if this might be an issue with WebAssembly in general, or maybe Pyodide.

In any case this would not be a blocker for this PR and could be investigated independently.

ivanistheone commented 2 years ago

Okay all PHASE 1 work done; @asmeurer @certik ready for review.

Summary of changes:

Screenshot: webcopy screenshot

If you have edit-suggestions for webcopy of sidebar, please edit the text in the google doc and I will update the html in templates/shell.html based on the gdoc.

ivanistheone commented 2 years ago

Were you able to get some information from the dev tools console? Curious if this might be an issue with WebAssembly in general, or maybe Pyodide.

@jtpio I managed to connect to get a Safari debug console from destkop; I put some screenshots of errors here: https://github.com/jupyterlite/jupyterlite/issues/632

asmeurer commented 2 years ago

On iOS Safari (latest iOS), I get this error

Traceback (most recent call last):

  File /lib/python3.10/site-packages/IPython/core/interactiveshell.py:3397 in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)

  Input In [1] in <cell line: 1>
    from sympy import *

  File /lib/python3.10/site-packages/sympy/__init__.py:73 in <module>
    from .polys import (Poly, PurePoly, poly_from_expr, parallel_poly_from_expr,

  File /lib/python3.10/site-packages/sympy/polys/__init__.py:75 in <module>
    from .polyfuncs import (symmetrize, horner, interpolate,

  File /lib/python3.10/site-packages/sympy/polys/polyfuncs.py:11 in <module>
    from sympy.polys.specialpolys import (

  File /lib/python3.10/site-packages/sympy/polys/specialpolys.py:7 in <module>
    from sympy.functions.elementary.miscellaneous import sqrt

  File /lib/python3.10/site-packages/sympy/functions/__init__.py:10 in <module>
    from sympy.functions.combinatorial.numbers import (carmichael, fibonacci, lucas, tribonacci,

  File /lib/python3.10/site-packages/sympy/functions/combinatorial/numbers.py:27 in <module>
    from sympy.functions.elementary.trigonometric import sin, cos, cot

  File /lib/python3.10/site-packages/sympy/functions/elementary/trigonometric.py:3302
    return sqrt(arg**2)/arg*(S.Pi/2 - acot(1/sqrt(arg**2 - 1)))
                                                             ^
SyntaxError: invalid syntax
asmeurer commented 2 years ago

What is the caching story here? For me, whenever I reload the page, it always takes a few seconds to load. Is the "loading" progress bar just tracking downloading packages or is it also including import time? I don't think we need to have one for import time.

Also, if it is possible, it would better to have the loading bar work in a way that lets people enter code while it is loading and have it run as soon as the loading happens.

asmeurer commented 2 years ago

Is it hard to change the submit button from shift-enter to just enter? I suppose that would break multiline code, unless it uses something similar to IPython where it adds a newline if the code is multiline and submits otherwise.

ivanistheone commented 2 years ago

On iOS Safari (latest iOS), I get this error Traceback (most recent call last): File /lib/python3.10/site-packages/sympy/functions/elementary/trigonometric.py:3302 return sqrt(arg*2)/arg(S.Pi/2 - acot(1/sqrt(arg**2 - 1))) SyntaxError: invalid syntax

I ran into a similar error in Opera on iOS and opened an issue here: https://github.com/jupyterlite/jupyterlite/issues/633


What is the caching story here?

Good question. I also get the same loading times whether I CTRL+R (refresh) or CTRL+SHIFT+R (clear cache and refresh). I defer to @jtpio to say if there is something we can do. Not a blocker for this PR, but would be nice to bring down the loading time in future updates.


Also, if it is possible, it would better to have the loading bar work in a way that lets people enter code while it is loading and have it run as soon as the loading happens.

The loading overlay is a complete hack (hard coded 18 sec delay). Basically, I figured since users have to wait O(20 sec) for the shell to become interactive, we better block them for some time to let them know it's loading. Should we reduce this to 10 seconds? (after the loading screen is gone, you can enter commands as per your suggestion)


Is it hard to change the submit button from shift-enter to just enter?

I was looking into that today. I created ivanistheone/live from the jupyterlite/demo template and I was looking into the various config options, but I don't know how to customize keypress yet.

jtpio commented 2 years ago

I was looking into that today. I created ivanistheone/live from the jupyterlite/demo template and I was looking into the various config options, but I don't know how to customize keypress yet.

@ivanistheone I opened https://github.com/ivanistheone/live/pull/1 to use Enter for executing code, and also enable some app optimizations by only deploying the repl app.

jtpio commented 2 years ago

but would be nice to bring down the loading time in future updates.

Yes I'm hoping we can add the import hooks back upstream in JupyterLite: https://github.com/jupyterlite/jupyterlite/issues/254 So packages like matplotlib would not be imported at startup, but only when needed. This should already help reduce the loading time.

ivanistheone commented 2 years ago

Update from this morning.

Thank's to Jeremy's PRs https://github.com/ivanistheone/live/pull/1, https://github.com/ivanistheone/live/pull/2, https://github.com/ivanistheone/live/pull/3, we now have custom JupyterLite with SUBMIT on ENTER key bindings (and COMMAND+ ENTER for newlines).

You can see it here: https://ivanistheone.github.io/live/repl/?toolbar=1&kernel=python but we want this to run here: https://sympy.github.io/live/repl/?toolbar=1&kernel=python before updating any links.

I would like to tansfer ownership of https://github.com/ivanistheone/live to https://github.com/sympy today, but I will need someone from the sympy org to give me collaborator access to the repo after the transfer so I can continue working on it.

@asmeurer @certik Could we do this today, if not I will continue working on ivanistheone for now.

asmeurer commented 2 years ago

The loading overlay is a complete hack (hard coded 18 sec delay). Basically, I figured since users have to wait O(20 sec) for the shell to become interactive, we better block them for some time to let them know it's loading. Should we reduce this to 10 seconds? (after the loading screen is gone, you can enter commands as per your suggestion)

Oh, I don't like this. No wonder it feels so slow. I think I'd rather have no loading indicator at all than one that is on a fixed timer. Especially if the user is able to enter code asynchronously with the loading. Will any code entered always run once everything is loaded, or does it have to be entered after a certain point.

Either way, I also think that better loading indicators can be considered an upstream problem for jupyterlite (hopefully @ivanistheone agrees), so I'm OK with not doing anything here and letting jupyterlite itself improve in this regard. Simply having some text in the instructions that it may take a few seconds to load should be sufficient for now.

I would like to tansfer ownership of ivanistheone/live to @sympy today, but I will need someone from the sympy org to give me collaborator access to the repo after the transfer so I can continue working on it.

@asmeurer @certik Could we do this today, if not I will continue working on ivanistheone for now.

Yes we can do it. What do I need to do?

asmeurer commented 2 years ago

I'm also happy to give you push access to this repo if that will help you to move this forward.

ivanistheone commented 2 years ago

I would like to tansfer ownership of ivanistheone/live to @sympy today, but I will need someone from the sympy org to give me collaborator access to the repo after the transfer so I can continue working on it.

Could we do this today, if not I will continue working on ivanistheone for now.

Yes we can do it. What do I need to do?

@asmeurer I have added you as a collaborator on https://github.com/ivanistheone/live To transfer to sympy org you must do (becasue I can't create repos on sympy). Steps:

  1. Go to https://github.com/ivanistheone/live/settings
  2. Click on "Transfer" button near bottom of the General settings
  3. Fill in form transfer steps
  4. Make sure I'm still collaborator on the repo after transfer (commit and github actions)
ivanistheone commented 2 years ago

Oh, I don't like this. No wonder it feels so slow. I think I'd rather have no loading indicator at all than one that is on a fixed timer. Especially if the user is able to enter code asynchronously with the loading. Will any code entered always run once everything is loaded, or does it have to be entered after a certain point.

Agree the fixed-length overlay is bad.

I tested and the JupyterLite UI takes 3-4 seconds to load, after that you can start entering commands (which will be executed in order when the loading completes). I'd say it's worth leaving a 3 seconds loading overlay, then hide it (down from the current 18 which I agree is bad).

Instructions in the side panel currently say "Note it can take up to 30 seconds before the shell loads and becomes available for using interactively." But they are not super visible right now. Maybe we can move them to the space above the shell, as shown below:

Screen Shot 2022-05-05 at 4 13 56 PM

I can then set up a 30-seconds-delayed jQuery action to .hide() that message so it won't be in the way...

How does that sound?

asmeurer commented 2 years ago

That should be better. If we can make the loading indicator actually tied to the loading instead of on a timer that would be best.

ivanistheone commented 2 years ago

That should be better. If we can make the loading indicator actually tied to the loading instead of on a timer that would be best.

Yes that't right, but in this case the REPL is running in an iframe so we can't set/get any "events" on it (would require postMessage API, but that's fancy stuff...

For the future live.sympy.org version we'll be running the actual repl (not in an iframe) so I'm sure there are places we can hook into (ideally not custom for us but "upstream" in jupyterlite).

asmeurer commented 2 years ago

Sorry, I think you need to make me an admin of the repo. I don't seem to have permissions to access the repo settings.

asmeurer commented 2 years ago

What does it look like if we don't have any loading indicator at all? 3 seconds isn't that long.

asmeurer commented 2 years ago

OK, I did the transfer. https://github.com/sympy/live. I made a new GitHub team that gives you write access to all the websites. Let me know if you need more access than that.

It looks like https://www.sympy.org/live/ currently redirects to https://www.sympy.org/live/lab/index.html which gives 404.

jtpio commented 2 years ago

It looks like https://www.sympy.org/live/ currently redirects to https://www.sympy.org/live/lab/index.html which gives 404.

This should not be an issue because we point to https://www.sympy.org/live/repl/index.html directly in this PR. But I opened https://github.com/sympy/live/pull/5 to handle this in case folks land on https://www.sympy.org/live from other places.

ivanistheone commented 2 years ago

Updates:

Demo link as usual: https://ivanistheone.github.io/sympy.github.com/en/shell.html

Waiting for further webcopy edits/suggestions and/or testing. We also discussed getting rid of the loading overlay completely, but I think it's useful to have and 3 secs is not long. If no further changes, then we ship it!

asmeurer commented 2 years ago

The disappearing message means the content shifts up. I would rather just have it below the content and not disappearing.

asmeurer commented 2 years ago

sympy.org/live now just redirects to the notebook rather than the whole page. If we are going to have that as a page, we should make it link to the actual live page, or redirect to live.sympy.org.

asmeurer commented 2 years ago

I still prefer to not have the loading overlay. I can see it load underneath the loading indicator before it finishes, meaning I have to wait 3 seconds even if I wouldn't have to. Having a loading indicator is fine, but my opinion is it would be better to have it in some way that does not disable access to the content.

asmeurer commented 2 years ago

I'm +1 for shipping this once you think it is ready. You should have push access to this repo.

ivanistheone commented 2 years ago

I still prefer to not have the loading overlay.

Gone.

The disappearing message means the content shifts up. I would rather just have it below the content and not disappearing.

Moved.

For the record, I think the disappearing message on top would have been better UX (more prominent to tell user what is going on), but I don't feel strongly about it.

removed dissapearing message on top

Fixed message below is good enough... and we can tweak it further if need be.

sympy.org/live now just redirects to the notebook rather than the whole page. If we are going to have that as a page, we should make it link to the actual live page, or redirect to live.sympy.org.

Is https://sympy.org/live linked to from somewhere currently? I've added a TODO to look into this in next phase.

ivanistheone commented 2 years ago

I think this one is ready to merge! I see the Merge pull request button is available to me, so I can merge myself if there are no further changes requested.

Demo here as usual: https://ivanistheone.github.io/sympy.github.com/en/shell.html

asmeurer commented 2 years ago

Yes, let's go ahead and merge this. We can make improvements as new pull requests.

ivanistheone commented 2 years ago

Okay merged.


When testing the evaluate? query strings, I noticed a problem with the redirect sympy.github.io --> www.sympy.org that was dropping repeated query string parameters. E.g. sympy.github.io?code=a&code=b will get redirected to www.sympy.org?code=a dropping the code=b.

I pushed a hotfix PR #175 changing the URL we use for the shell to be www.sympy.org so now there are no more redirects and functionality works as expected.


On an unrelated note: I noticed a non-critical error in the JS console (it doesn't seem to break any functionality; I opened an issue for it on jupyterlite repo https://github.com/jupyterlite/jupyterlite/issues/635 in case want to track it down)

asmeurer commented 2 years ago

Great. Can you send an announcement to the mailing list?

ivanistheone commented 2 years ago

Can you send an announcement to the mailing list?

I was going to send an email announcement + request for testing, but I ran into an issue https://github.com/jupyterlite/jupyterlite/issues/636 so I won't post today. Hopefully it's just a transient error.

I will send announcement when fixed.

Flagging https://github.com/jupyterlite/jupyterlite/issues/636 for @jtpio to triage appropriately.

asmeurer commented 2 years ago

OK. We don't need to have things in a perfect state to send an announcement about the soft launch, especially since the site is live anyway, so people are already going to start going to it. And it was already announced on Twitter.

ivanistheone commented 2 years ago

Issue#636 was transient; everything works fine now.

I sent the announcement to the mailing list.

Thanks everyone who has been part of this so far. I'm very glad we're progressing toward latest-SymPy-live-shell-in-the-browser-with-almost-no-maintenance future.

asmeurer commented 2 years ago

I haven't heard any negative feedback on this yet. I guess we should move live.sympy.org to it as well. Then once we do a SymPy release with the new Sphinx theme that removes the Live extension, we can shut Live down (or at least remove the billing).

asmeurer commented 2 years ago

Maybe doing this just requires updating the DNS for live.sympy.org?

asmeurer commented 2 years ago

For example, here is how it can be done with namecheap:

Screen Shot 2022-07-26 at 3 12 47 PM

See http://live.asmeurer.com/. Except I don't know how to make it use https. I think NumFOCUS handles the sympy.org DNS. @certik do you know what needs to be done to change this?

ivanistheone commented 2 years ago

Hi @asmeurer ; Is there a reason you're looking for a new plan? The previously agreed upon approach was to wait to use the https://sympy.org/en/shell.html for a testing period, then deploy the repo https://github.com/sympy/live as github pages then point live.sympy.org to the github pages (PHASE4).

I haven't pushed on this recently because I've asked several people to test https://sympy.org/en/shell.html and I've had very few good reviews. Desktop browsers work fine, but it doesn't seem to be usable on mobile (not loading at all on older browsers and takes too long on modern). This seemed like a show stopper to me, that's why I haven't pushed on the migration recently...

Perhaps we can agree that supporting mobile users and legacy browsers are out of scope? If we agree on that then, I can resume work on the roadmap we discussed before: PHASE3 and PHASE4.

Also, I'm curious. Dos someone know what's the current GAE hosting bill for running live.sympy.org ? Is there a way to access server logs to see what percentage of user-agents are mobile phones? (perhaps there are not so many mobile users, so it won't be letting anyone down).

asmeurer commented 2 years ago

I'm not pushing for a new plan. But I do think this has been live long enough that we can consider the testing period to be over.

It looks like Live is costing us about $20-30 a month (Gamma is much more). It's hard to tell, but I think the costs have declined a little bit since deploying this, but not a whole lot. I expect most users of Live are coming from the docs.

The reason I am pushing for this is that I want to shut down Live. As soon as we do the next SymPy release (which will be soon), it will remove the Live extension from the docs, so the only thing remaining will be live.sympy.org.

I have full access to GAE, but I have no idea how to extract that information from the logs (the Google Cloud console is very confusing). I think the information is there (user agent), but I don't know how to easily extract it and I don't really have the time to figure it out. I'm happy to give you permissions on GAE if you want to look into it yourself.