sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.43k stars 478 forks source link

Implement SVG plotting in pari_jupyter #22177

Closed jdemeyer closed 7 years ago

jdemeyer commented 7 years ago

This requires various patches to upstream PARI:

The pari_jupyter package also needs to be updated to support SVG plotting:

Demo: https://github.com/OpenDreamKit/OpenDreamKit/blob/master/WP4/D4.4/demo-pari.ipynb

Tarball: https://pypi.python.org/packages/6d/a7/9186eeacf76f8b723915b9c745ab42a73b987e5b5296fdd4a0a7c6626800/pari_jupyter-1.2.2.tar.bz2

Depends on #22276

Upstream: Reported upstream. Developers acknowledge bug.

CC: @dimpase

Component: packages: standard

Author: Jeroen Demeyer

Branch/Commit: 7df14aa

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/22177

jdemeyer commented 7 years ago

Changed dependencies from #21756 to #21756, #22180

jdemeyer commented 7 years ago

Changed dependencies from #21756, #22180 to #22180, #22276

jdemeyer commented 7 years ago

Branch: u/jdemeyer/implement_svg_plotting_in_pari_jupyter

jdemeyer commented 7 years ago

New commits:

a70bf47Fix and add PARI patches
72c15e4Add patches to PARI for SVG plotting
jdemeyer commented 7 years ago

Commit: 72c15e4

jdemeyer commented 7 years ago

Changed dependencies from #22180, #22276 to #22276

jdemeyer commented 7 years ago

Description changed:

--- 
+++ 
@@ -1 +1,11 @@
+This adds various patches to upstream PARI:

+- `pari_str_1.patch` and `pari_str_2.patch`: implement new `pari_str` API. Patch taken from upstream master.
+
+- `plot_libpari.patch`: move plotting interface from gp to libPARI. Being discussed with upstream.
+
+- `plot_svg.patch`: implement SVG plotting libPARI. Applied upstream with significant changes.
+
+The `pari_jupyter` package also needs to be updated to support SVG plotting:
+
+**Tarball**: https://pypi.python.org/packages/6d/a7/9186eeacf76f8b723915b9c745ab42a73b987e5b5296fdd4a0a7c6626800/pari_jupyter-1.2.2.tar.bz2
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

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

7df14aaUpgrade pari_jupyter
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 72c15e4 to 7df14aa

dimpase commented 7 years ago
comment:8

not sure I am really qualified to review this; could you at least provide a jupyter notebook with the graphics in question, to test if it works?

jdemeyer commented 7 years ago

Upstream: Reported upstream. Developers acknowledge bug.

jdemeyer commented 7 years ago

Description changed:

--- 
+++ 
@@ -1,10 +1,10 @@
-This adds various patches to upstream PARI:
+This requires various patches to upstream PARI:

 - `pari_str_1.patch` and `pari_str_2.patch`: implement new `pari_str` API. Patch taken from upstream master.

 - `plot_libpari.patch`: move plotting interface from gp to libPARI. Being discussed with upstream.

-- `plot_svg.patch`: implement SVG plotting libPARI. Applied upstream with significant changes.
+- `plot_svg.patch`: implement SVG plotting in libPARI. Applied upstream with significant changes.

 The `pari_jupyter` package also needs to be updated to support SVG plotting:
jdemeyer commented 7 years ago
comment:10

Replying to @dimpase:

not sure I am really qualified to review this; could you at least provide a jupyter notebook with the graphics in question, to test if it works?

See https://github.com/OpenDreamKit/OpenDreamKit/blob/master/WP4/D4.4/demo-pari.ipynb

jdemeyer commented 7 years ago

Description changed:

--- 
+++ 
@@ -8,4 +8,6 @@

 The `pari_jupyter` package also needs to be updated to support SVG plotting:

+**Demo**: https://github.com/OpenDreamKit/OpenDreamKit/blob/master/WP4/D4.4/demo-pari.ipynb
+
 **Tarball**: https://pypi.python.org/packages/6d/a7/9186eeacf76f8b723915b9c745ab42a73b987e5b5296fdd4a0a7c6626800/pari_jupyter-1.2.2.tar.bz2
dimpase commented 7 years ago
comment:12

the notebook looks good, besides ellinit("10a1") needs some extra data I don't have installed


  ***   at top-level: ellinit("10a1")
  ***                 ^---------------
  *** ellinit: error opening elldata file: `/home/dima/Sage/sage-dev/local/share/pari/elldata/ell0'.

(well, this is not relevant to this ticket).

Tonight I'll see how this goes on OSX, and hopefully it's good to go then.

jdemeyer commented 7 years ago
comment:13

Replying to @dimpase:

the notebook looks good, besides ellinit("10a1") needs some extra data I don't have installed

As documented in the demo notebook, that's on purpose, to show that error handling works fine. There is no curve called 10a1, so installing a database won't help.

dimpase commented 7 years ago

Reviewer: Dima Pasechnik

dimpase commented 7 years ago
comment:14

looks good on OSX too.

vbraun commented 7 years ago

Changed branch from u/jdemeyer/implement_svg_plotting_in_pari_jupyter to 7df14aa