sagemath / sage

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

SAGERUNTIME requires future and pyparsing #22126

Closed jdemeyer closed 7 years ago

jdemeyer commented 7 years ago

Sage now contains various lines of the form from builtins import ... which is provided by the future package.

Currently, pyparsing is needed for src/sage/homology/simplicial_set_examples.py, which we solve with a lazy import.

CC: @jhpalmieri

Component: build

Author: Jeroen Demeyer

Branch/Commit: 98358f7

Reviewer: John Palmieri

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

jdemeyer commented 7 years ago

Description changed:

--- 
+++ 
@@ -1 +1 @@
-
+Sage now contains various lines of the form `from builtins import ...` which is provided by the `future` package.
jdemeyer commented 7 years ago

Branch: u/jdemeyer/sageruntime_requires_future

jdemeyer commented 7 years ago

Commit: ea36a9b

jdemeyer commented 7 years ago

New commits:

ea36a9bSAGERUNTIME requires future
fchapoton commented 7 years ago
comment:4

and what about six ? currently, trying to build with SAGE_PYTHON3=yes fails on trying to import six...

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

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

eceafc7Add a "sageruntime" build target
44e5534Update SAGERUNTIME dependencies
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from ea36a9b to 44e5534

jdemeyer commented 7 years ago
comment:6

Rebase to 7.6.rc1 and updated (now also pyparsing is needed).

six is indirectly installed as dependency of IPython. It is not needed to explicitly add it as dependency of SAGERUNTIME.

jdemeyer commented 7 years ago

Description changed:

--- 
+++ 
@@ -1 +1 @@
-Sage now contains various lines of the form `from builtins import ...` which is provided by the `future` package.
+Sage now contains various lines of the form `from builtins import ...` which is provided by the `future` package. Also `pyparsing` is needed for `src/sage/homology/simplicial_set_examples.py`.
fchapoton commented 7 years ago
comment:7

ok. But I would appreciate if somebody could figure (elsewhere) why python3 build is currently broken on six import. This is beyond my meager abilities..

jdemeyer commented 7 years ago
comment:8

Replying to @fchapoton:

ok. But I would appreciate if somebody could figure (elsewhere) why python3 build is currently broken on six import. This is beyond my meager abilities..

I might have a look later...

Could it be that some package requires six in Python 3 but not in Python 2 (that would be surprising, but it would explain things).

jdemeyer commented 7 years ago
comment:9

Replying to @fchapoton:

ok. But I would appreciate if somebody could figure (elsewhere) why python3 build is currently broken on six import. This is beyond my meager abilities..

See #22638

jhpalmieri commented 7 years ago
comment:10

If we can lazily import simplicial sets instead of adding this runtime dependency, that would be fine with me.

jhpalmieri commented 7 years ago
comment:11

For example, can we just do

diff --git a/src/sage/homology/all.py b/src/sage/homology/all.py
index 511ad4d..f26eb8c 100644
--- a/src/sage/homology/all.py
+++ b/src/sage/homology/all.py
@@ -16,4 +16,4 @@ from .cubical_complex import CubicalComplex, cubical_complexes
 from sage.misc.lazy_import import lazy_import
 lazy_import('sage.homology.koszul_complex', 'KoszulComplex')

-from . import simplicial_set_catalog as simplicial_sets
+lazy_import('sage.homology', 'simplicial_set_catalog', _as='simplicial_sets')

?

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

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

f835414Update SAGERUNTIME dependencies
769d635Lazy import catalogs in homology
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 44e5534 to 769d635

jdemeyer commented 7 years ago

Reviewer: John Palmieri

jdemeyer commented 7 years ago
comment:14

Replying to @jhpalmieri:

If we can lazily import simplicial sets instead of adding this runtime dependency, that would be fine with me.

Done, please review.

jdemeyer commented 7 years ago

Description changed:

--- 
+++ 
@@ -1 +1,3 @@
-Sage now contains various lines of the form `from builtins import ...` which is provided by the `future` package. Also `pyparsing` is needed for `src/sage/homology/simplicial_set_examples.py`.
+Sage now contains various lines of the form `from builtins import ...` which is provided by the `future` package.
+
+Currently, `pyparsing` is needed for `src/sage/homology/simplicial_set_examples.py`, which we solve with a lazy import.
jhpalmieri commented 7 years ago
comment:15

I get an unpickling failure:

sage -t --long --warn-long 74.2 src/sage/structure/sage_object.pyx
**********************************************************************
File "src/sage/structure/sage_object.pyx", line 1526, in sage.structure.sage_object.unpickle_all
Failed example:
    sage.structure.sage_object.unpickle_all()  # (4s on sage.math, 2011)
Expected:
    doctest:... DeprecationWarning: ...
    See http://trac.sagemath.org/... for details.
    Successfully unpickled ... objects.
    Failed to unpickle 0 objects.
Got:
    doctest:warning

[snip]

    Failed:
    _class__sage_homology_examples_SimplicialSurface__.sobj
    ----------------------------------------------------------------------
    ** This error is probably due to an old pickle failing to unpickle.
    ** See sage.structure.sage_object.register_unpickle_override for
    ** how to override the default unpickling methods for (old) pickles.
    ** NOTE: pickles should never be removed from the pickle_jar! 
    ----------------------------------------------------------------------
    Successfully unpickled 585 objects.
    Failed to unpickle 1 objects.
**********************************************************************
1 item had failures:
   1 of  11 in sage.structure.sage_object.unpickle_all
    [211 tests, 1 failure, 7.91 s]
----------------------------------------------------------------------
sage -t --long --warn-long 74.2 src/sage/structure/sage_object.pyx  # 1 doctest failed
----------------------------------------------------------------------

This is due to the change in importing the simplicial complex catalog, which I think is not necessary for this ticket.

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

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

98358f7Move unpickle override to all.py
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 769d635 to 98358f7

jdemeyer commented 7 years ago
comment:17

Replying to @jhpalmieri:

This is due to the change in importing the simplicial complex catalog, which I think is not necessary for this ticket.

True, it is not needed. However, I did it by analogy with the other catalog. Anyway, the pickling problem is easily solved by moving the unpickle_override to the file all.py which is always imported.

jhpalmieri commented 7 years ago
comment:18

Looks good, thanks.

vbraun commented 7 years ago

Changed branch from u/jdemeyer/sageruntime_requires_future to 98358f7