downgrade various sagenb deps to optional

[dimpase]

This includes twisted and flask* packages.

CC: @slel @kiwifb

Component: packages: standard

Keywords: sagenb, dependencies, optional

Author: Frédéric Chapoton, John Palmieri

Branch: d93d9d1

Reviewer: Dima Pasechnik

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

[fchapoton]

Commit: 89d2a99

[fchapoton]

comment:1

here is a branch (not tested)

---

New commits:

89d2a99

change twisted and flask_* to optional packages

[fchapoton]

Branch: u/chapoton/29320

[dimpase]

Changed author from Dima Pasechnik to none

[dimpase]

Reviewer: Dima Pasechnik

[dimpase]

comment:2

You beat me by 5 minutes :-)
But I've tested that this works.

[dimpase]

Author: Frédéric Chapoton

[slel]

Changed keywords from none to sagenb, dependencies, optional

[jhpalmieri]

comment:5

@kiwifb told me that terminado uses twisted, so maybe twisted should be kept as standard. (Also, it should be listed in terminado's dependencies file, if this is accurate.) Can anyone confirm?

[kiwifb]

comment:6

I should have said tornado, I misread the output of my dependency query in gentoo. That being said terminado depends on tornado.

[dimpase]

comment:7

the tests pass, and I manually checked dependencies to make sure that this change leads to no standard packages missing deps. If there are undeclared deps of untested standard packages, it is a bug.

[antonio-rojas]

comment:8

Other candidates for downgrade: python_openid, speaklater

[dimpase]

comment:9

I think we can downgrade tornado and terminado etc. I don't know who is using them.

[jhpalmieri]

comment:10

The Jupyter notebook may use these if they are available, but I don't know any details.

[antonio-rojas]

comment:11

Replying to @jhpalmieri:

The Jupyter notebook may use these if they are available, but I don't know any details.

They are actually mandatory https://github.com/jupyter/notebook/blob/master/setup.py#L99

[jhpalmieri]

comment:12

Twisted, on the other hand, looks optional: see "Prerequisites" in https://www.tornadoweb.org/. Twisted is only used in tornado.platform.twisted: "This module lets you run applications and libraries written for Twisted in a Tornado application."

[jhpalmieri]

comment:13

So it looks to me like python_openid, speaklater, and twisted could all be removed. Re twisted especially, we should not only build and run tests, but also make sure the Jupyter notebook works.

[jhpalmieri]

comment:14

Replying to @dimpase:

the tests pass

Not for me:

$ ./sage -t src/sage/combinat/root_system/cartan_type.py
Running doctests with ID 2020-03-13-16-27-46-31297653.
Git branch: t/29320/29320
Using --optional=build,dochtml,sage
Doctesting 1 file.
sage -t --warn-long 69.1 src/sage/combinat/root_system/cartan_type.py
**********************************************************************
File "src/sage/combinat/root_system/cartan_type.py", line 3069, in sage.combinat.root_system.cartan_type.CartanType_simple_finite.__setstate__
Failed example:
    pg_unpickleModule = unpickle_global('twisted.persisted.styles', 'unpickleModule')
Exception raised:
    Traceback (most recent call last):
      File "sage/misc/persist.pyx", line 548, in sage.misc.persist.unpickle_global (build/cythonized/sage/misc/persist.c:5033)
        __import__(module)
    ModuleNotFoundError: No module named 'twisted'

...

[jhpalmieri]

comment:15

In brief testing, though, the Jupyter notebook looks okay.

[jhpalmieri]

comment:16

I propose these changes:

diff --git a/build/pkgs/python_openid/type b/build/pkgs/python_openid/type
index a6a7b9cd72..134d9bc32d 100644
--- a/build/pkgs/python_openid/type
+++ b/build/pkgs/python_openid/type
@@ -1 +1 @@
-standard
+optional
diff --git a/build/pkgs/speaklater/type b/build/pkgs/speaklater/type
index a6a7b9cd72..134d9bc32d 100644
--- a/build/pkgs/speaklater/type
+++ b/build/pkgs/speaklater/type
@@ -1 +1 @@
-standard
+optional
diff --git a/build/pkgs/twisted/type b/build/pkgs/twisted/type
index 134d9bc32d..a6a7b9cd72 100644
--- a/build/pkgs/twisted/type
+++ b/build/pkgs/twisted/type
@@ -1 +1 @@
-optional
+standard

[jhpalmieri]

comment:17

Replying to @jhpalmieri:

Replying to @dimpase:

the tests pass

Not for me:

$ ./sage -t src/sage/combinat/root_system/cartan_type.py
...

Admittedly, this is a doctest for a sort of stupid function: "Implements the unpickling of Cartan types pickled by Sage <= 4.0."

[dimpase]

comment:18

Replying to @jhpalmieri:

Replying to @jhpalmieri:

Replying to @dimpase:

the tests pass

Not for me:

$ ./sage -t src/sage/combinat/root_system/cartan_type.py
...

Admittedly, this is a doctest for a sort of stupid function: "Implements the unpickling of Cartan types pickled by Sage <= 4.0."

Oops, I overlooked this, I thought that this must be irrelevant.

Let's just tag it # optional - twisted. I don't see a need to keep twisted standard just cause of this.

[jhpalmieri]

Changed branch from u/chapoton/29320 to u/jhpalmieri/29320

[jhpalmieri]

comment:20

Okay, ready for review. Please check the functionality of the Jupyter notebook; do we lose anything when we build without twisted?

---

New commits:

d93d9d1

trac 29320: make python_openid and speaklater optional.

[jhpalmieri]

Changed commit from 89d2a99 to d93d9d1

[seblabbe]

comment:22

Looking at:

$ cat build/pkgs/sagenb/dependencies 
$(STARTED) | pip babel flask flask_autoindex flask_babel flask_oldsessions flask_openid mathjax twisted sphinx

----------
All lines of this file are ignored except the first.
It is copied by SAGE_ROOT/build/make/install into SAGE_ROOT/build/make/Makefile.

Are there other dependencies to be removed from being standard? like babel ?

[kiwifb]

comment:23

babel should actually be in the list removed from standard I believe. Thanks for bringing it up. twisted seems a bit ambiguous but mathjax and sphinx should definitely stay standard.

[jhpalmieri]

comment:24

babel is listed as a dependency of Sphinx, so it should be kept.

[kiwifb]

comment:25

Replying to @jhpalmieri:

babel is listed as a dependency of Sphinx, so it should be kept.

Ah! Sorry, my mistake.

[dimpase]

comment:26

Looks good, but we need to take care of the dependency of that pickle on twisted (on a followup ticket).

[dimpase]

Changed author from Frédéric Chapoton to Frédéric Chapoton, John Palmieri

[jhpalmieri]

comment:27

I don't understand pickling, but I am tempted to make this change:

diff --git a/src/sage/combinat/root_system/cartan_type.py b/src/sage/combinat/root_system/cartan_type.py
index 202e1abe11..3cac952462 100644
--- a/src/sage/combinat/root_system/cartan_type.py
+++ b/src/sage/combinat/root_system/cartan_type.py
@@ -3066,10 +3066,10 @@ class CartanType_simple_finite(object):
 
             sage: pg_CartanType_simple_finite = unpickle_global('sage.combinat.root_system.cartan_type', 'CartanType_simple_finite')
             sage: si1 = unpickle_newobj(pg_CartanType_simple_finite, ())
-            sage: pg_unpickleModule = unpickle_global('twisted.persisted.styles', 'unpickleModule')
+            sage: from sage.misc.fpickle import unpickleModule
             sage: pg_make_integer = unpickle_global('sage.rings.integer', 'make_integer')
             sage: si2 = pg_make_integer('4')
-            sage: unpickle_build(si1, {'tools':pg_unpickleModule('sage.combinat.root_system.type_A'), 't':['A', si2], 'letter':'A', 'n':si2})
+            sage: unpickle_build(si1, {'tools':unpickleModule('sage.combinat.root_system.type_A'), 't':['A', si2], 'letter':'A', 'n':si2})
 
             sage: si1
             ['A', 4]

I'm building and running tests now.

[jhpalmieri]

comment:28

See #29348 for the pickling issue.

[vbraun]

Changed branch from u/jhpalmieri/29320 to d93d9d1

[jhpalmieri]

Changed commit from d93d9d1 to none

[jhpalmieri]

comment:30

Followup at #29752: werkzeug can be optional, too.