Opened 2 years ago
Closed 2 years ago
#23706 closed enhancement (fixed)
allow several implementations of matrices in MatrixSpace
Reported by:  vdelecroix  Owned by:  

Priority:  major  Milestone:  sage8.2 
Component:  linear algebra  Keywords:  days88 
Cc:  jipilab, SimonKing  Merged in:  
Authors:  Vincent Delecroix  Reviewers:  JeanPhilippe Labbé, Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  018226b (Commits)  Commit:  018226b9ecafdc082a4eb8289269af85b2908d3b 
Dependencies:  #24096  Stopgaps: 
Description (last modified by )
The aim of this ticket is to revamp the MatrixSpace
constructor in order to be able to choose the implementation.
For example, with the attached branch one can have access to 2 implementations of integer matrices
sage: MatrixSpace(ZZ, 3, implementation='flint') Full MatrixSpace of 3 by 3 dense matrices over Integer Ring sage: MatrixSpace(ZZ, 3, implementation='generic') Full MatrixSpace of 3 by 3 dense matrices over Integer Ring (using Matrix_generic_dense)
One important change is the move of the method _get_matrix_class
of MatrixSpace
as an independent function.
A problem arose with CartanMatrix
(in sage/combinat/root_system/cartan_matrix.py
). The class CartanMatrix
inherits from Matrix_integer_sparse
but does not properly redefines the parent. As a consequence of the changes here, many matrix operations are not valid anymore for inherited types (mostly submatrices operations such as __getitem__
with slices or stack
). A fix is provided by defining CartanMatrix.matrix_space
.
follow up: #23714 (implementation of gap matrices)
Change History (70)
comment:1 Changed 2 years ago by
 Description modified (diff)
comment:2 Changed 2 years ago by
 Keywords sd88 added
comment:3 Changed 2 years ago by
 Keywords days88 added; sd88 removed
comment:4 Changed 2 years ago by
 Branch set to u/vdelecroix/23706
 Commit set to 34f1ddc7e0ea69ab280567bf68da69d5fb2f0cd2
comment:5 Changed 2 years ago by
 Reviewers set to JeanPhilippe Labbé
The title of the ticket could be a bit more specific.
comment:6 Changed 2 years ago by
 Commit changed from 34f1ddc7e0ea69ab280567bf68da69d5fb2f0cd2 to d9f0532af4ca79ba467e466c741416b4409ca960
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
d9f0532  TMP 23706 commit

comment:7 Changed 2 years ago by
 Commit changed from d9f0532af4ca79ba467e466c741416b4409ca960 to 007bdcf7710533d0fd413c56d9d566b84db1873f
comment:8 Changed 2 years ago by
 Commit changed from 007bdcf7710533d0fd413c56d9d566b84db1873f to f8940d448bddc312f64ad8707cec630abf178b08
Branch pushed to git repo; I updated commit sha1. New commits:
f8940d4  TMP 23706: remove print statement

comment:9 Changed 2 years ago by
 Commit changed from f8940d448bddc312f64ad8707cec630abf178b08 to 235c5615b9dd35891a5ff5d598884093b152520b
Branch pushed to git repo; I updated commit sha1. New commits:
235c561  TMP 23706 COMMIT: remove randomize for cyclo_dense

comment:10 Changed 2 years ago by
 Commit changed from 235c5615b9dd35891a5ff5d598884093b152520b to dce22c869eee2d2b4a6f59cd9f2e6b5eda633121
Branch pushed to git repo; I updated commit sha1. New commits:
dce22c8  TMP COMMIT 23706: more careful about input

comment:11 Changed 2 years ago by
 Description modified (diff)
 Summary changed from gap class for matrices to allow several implementations of matrices in MatrixSpace
comment:12 Changed 2 years ago by
 Description modified (diff)
comment:13 Changed 2 years ago by
 Commit changed from dce22c869eee2d2b4a6f59cd9f2e6b5eda633121 to f1013de2bc8d01830840422c367dbe9224d0aeb6
comment:14 Changed 2 years ago by
 Dependencies set to #23704
 Status changed from new to needs_review
comment:15 Changed 2 years ago by
 Description modified (diff)
comment:16 Changed 2 years ago by
 Commit changed from f1013de2bc8d01830840422c367dbe9224d0aeb6 to 0cbf7ff5a1e4ec51bfd4251e7a0677b77ec945ce
Branch pushed to git repo; I updated commit sha1. New commits:
0cbf7ff  23706: fix call to _get_matrix_class

comment:17 Changed 2 years ago by
hum... got a lot of failures
combinat/root_system/root_lattice_realizations.py # 1 doctest failed combinat/root_system/root_system.py # 1 doctest failed combinat/root_system/weight_space.py # 3 doctests failed combinat/root_system/type_affine.py # 3 doctests failed combinat/root_system/ambient_space.py # 1 doctest failed combinat/root_system/root_space.py # 1 doctest failed combinat/root_system/cartan_type.py # 2 doctests failed categories/crystals.py # 2 doctests failed combinat/root_system/weight_lattice_realizations.py # 12 doctests failed combinat/root_system/cartan_matrix.py # 5 doctests failed combinat/root_system/type_relabel.py # 1 doctest failed combinat/root_system/dynkin_diagram.py # 4 doctests failed combinat/root_system/type_marked.py # 1 doctest failed combinat/root_system/type_reducible.py # 2 doctests failed
comment:18 Changed 2 years ago by
 Dependencies changed from #23704 to #23704, #23721
 Status changed from needs_review to needs_work
Needs to fix #23721 first.
comment:19 Changed 2 years ago by
 Commit changed from 0cbf7ff5a1e4ec51bfd4251e7a0677b77ec945ce to 9f74040baa887b1ff6e286cbccb7af77f364c6f3
comment:20 Changed 2 years ago by
 Dependencies #23704, #23721 deleted
 Description modified (diff)
comment:21 Changed 2 years ago by
 Status changed from needs_work to needs_review
I postponed the introduction of Matrix_gap
in ticket #23714
comment:23 Changed 2 years ago by
 Commit changed from 9f74040baa887b1ff6e286cbccb7af77f364c6f3 to 4efbf513a230daf4a39316149eedea94282ee335
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4efbf51  23706: allow to choose matrix mplementation

comment:24 Changed 2 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
comment:25 Changed 2 years ago by
JP, do you feel up for the review?
comment:26 Changed 2 years ago by
Fucking root systems
sage t long src/sage/combinat/root_system/cartan_type.py # 2 doctests failed sage t long src/sage/combinat/root_system/weight_lattice_realizations.py # 12 doctests failed sage t long src/sage/combinat/root_system/ambient_space.py # 1 doctest failed sage t long src/sage/combinat/root_system/root_lattice_realizations.py # 1 doctest failed sage t long src/sage/combinat/root_system/type_marked.py # 1 doctest failed sage t long src/sage/combinat/root_system/type_reducible.py # 2 doctests failed sage t long src/sage/combinat/root_system/root_space.py # 1 doctest failed sage t long src/sage/combinat/root_system/cartan_matrix.py # 5 doctests failed sage t long src/sage/combinat/root_system/dynkin_diagram.py # 4 doctests failed sage t long src/sage/combinat/root_system/weight_space.py # 3 doctests failed sage t long src/sage/combinat/root_system/type_relabel.py # 1 doctest failed sage t long src/sage/combinat/root_system/type_affine.py # 3 doctests failed sage t long src/sage/combinat/root_system/root_system.py # 1 doctest failed sage t long src/sage/categories/crystals.py # 2 doctests failed
comment:27 Changed 2 years ago by
 Commit changed from 4efbf513a230daf4a39316149eedea94282ee335 to d5fa13d8e9c3b44b549873810299b256fa7b6123
Branch pushed to git repo; I updated commit sha1. New commits:
d5fa13d  23706: dirty fixes for Cartan matrices

comment:28 Changed 2 years ago by
 Description modified (diff)
comment:29 Changed 2 years ago by
 Commit changed from d5fa13d8e9c3b44b549873810299b256fa7b6123 to 98d9ae1b64aac938ccb96b1b4f699b44b434bbf5
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
98d9ae1  23706: fix for CartanMatrix

comment:30 Changed 2 years ago by
Cartan matrices are fixed... waiting for patchbot verdict again.
comment:31 Changed 2 years ago by
 Description modified (diff)
comment:32 followup: ↓ 40 Changed 2 years ago by
Ok, I'm "back on trac"!
I'll give a look at this today.
Where should I throw in the benchmarking script? I guess it is not a good idea to hard code the best implementation, but at least, in the tests, we could add some relevant benchmarks to keep track of the evolution?
comment:33 Changed 2 years ago by
In the description of the test:
+ Check that multiplication for matrices with different backends are not allowed::
I would also mention that sparsity is checked ( the reason why there is no X appearing in row 2, column 3...)
The changes in the CartanMatrix? look good.
In one of the bot's report, I get an error here:
********************************************************************** File "src/sage/matrix/matrix_gfpn_dense.pyx", line 1484, in sage.matrix.matrix_gfpn_dense.Matrix_gfpn_dense._echelon_in_place_classical Failed example: type(X) Expected: <type 'sage.matrix.matrix_generic_dense.Matrix_generic_dense'> Got: <type 'sage.matrix.matrix_gfpn_dense.Matrix_gfpn_dense'> **********************************************************************
Other errors look to be unrelated. I'm still looking into the changes.
comment:34 followup: ↓ 37 Changed 2 years ago by
I ran the test on the file src/sage/matrix/matrix_gfpn_dense.pyx
and all tests passed...
comment:35 Changed 2 years ago by
 Branch changed from u/vdelecroix/23706 to u/jipilab/23706
comment:36 Changed 2 years ago by
 Commit changed from 98d9ae1b64aac938ccb96b1b4f699b44b434bbf5 to ae0bfc33a476dcf8ffe6e14808b4f36c2a2bf9c9
Just did some pep8 corrections... Otherwise, it looks good. Perhaps we should wait to get a proper bot to go over the changes as they touch many files...
New commits:
ae0bfc3  Some pep8 conventions

comment:37 in reply to: ↑ 34 Changed 2 years ago by
Replying to jipilab:
I ran the test on the file
src/sage/matrix/matrix_gfpn_dense.pyx
and all tests passed...
This need to be fixed. It depends whether meataxe
is installed or not.
comment:38 Changed 2 years ago by
 Branch changed from u/jipilab/23706 to public/23706
 Commit changed from ae0bfc33a476dcf8ffe6e14808b4f36c2a2bf9c9 to cc35c82b9d07517be8af21f67dbca33241cda6ac
New commits:
cc35c82  23706: fix doctest in matrix_gfpn_dense.pyx

comment:39 Changed 2 years ago by
Much cleaner now!!
comment:40 in reply to: ↑ 32 Changed 2 years ago by
Replying to jipilab:
Ok, I'm "back on trac"!
I'll give a look at this today.
Where should I throw in the benchmarking script? I guess it is not a good idea to hard code the best implementation, but at least, in the tests, we could add some relevant benchmarks to keep track of the evolution?
Not here. You should open a new ticket (and you can add it to the followup list of this ticket). Moreover the concept of "best implementation" depends on what you intend to do (e.g. matrix multiplication, Gauss elimination, determinant, etc).
comment:41 followup: ↓ 44 Changed 2 years ago by
The ticket does not apply on beta5. I'm compiling b5 right now and I'll check if I can diagnose further. I have to install meataxe
and checkout testing. Give my machine some time and I get back here!
comment:42 followup: ↓ 43 Changed 2 years ago by
Is there a dependency to #23721?
comment:43 in reply to: ↑ 42 Changed 2 years ago by
comment:44 in reply to: ↑ 41 ; followup: ↓ 46 Changed 2 years ago by
Replying to jipilab:
The ticket does not apply on beta5. I'm compiling b5 right now and I'll check if I can diagnose further. I have to install
meataxe
and checkout testing. Give my machine some time and I get back here!
The problem comes from something that has changed in matrix_space.py
. The merge is simple but I have to leave in 5 minutes... will fix that first thing this afternoon.
comment:45 Changed 2 years ago by
 Commit changed from cc35c82b9d07517be8af21f67dbca33241cda6ac to 869b155d053a8f4cb8d69d2575328b2d3ceabc86
Branch pushed to git repo; I updated commit sha1. New commits:
869b155  Rebased on beta5 (attempt to solve conflict)

comment:46 in reply to: ↑ 44 Changed 2 years ago by
Replying to vdelecroix:
Replying to jipilab:
The ticket does not apply on beta5. I'm compiling b5 right now and I'll check if I can diagnose further. I have to install
meataxe
and checkout testing. Give my machine some time and I get back here!The problem comes from something that has changed in
matrix_space.py
. The merge is simple but I have to leave in 5 minutes... will fix that first thing this afternoon.
Damn, I just read this comment... Hope I did not create any trouble.
comment:47 Changed 2 years ago by
It's ok.
comment:48 Changed 2 years ago by
There are some spurious doc errors, otherwise, it looks good.
comment:49 Changed 2 years ago by
 Commit changed from 869b155d053a8f4cb8d69d2575328b2d3ceabc86 to 079981ab5ca646a65072815b7b956060b908f5f7
comment:50 Changed 2 years ago by
j'ai poussé... reste plus qu'à attendre les verdicts de madame patchbot.
comment:51 Changed 2 years ago by
 Branch changed from public/23706 to u/vdelecroix/23706
 Commit changed from 079981ab5ca646a65072815b7b956060b908f5f7 to 5c415f5070e1e4cfb0e57c3c99cdd1470114e2ed
comment:53 Changed 2 years ago by
 Commit changed from 5c415f5070e1e4cfb0e57c3c99cdd1470114e2ed to 4f818deff90d1db47f55603509d22f0c267fcf8a
comment:54 Changed 2 years ago by
rebased on 8.1.rc0
comment:55 Changed 2 years ago by
patchbot is happy! jipilab?
comment:56 followup: ↓ 58 Changed 2 years ago by
 Cc SimonKing added
 Reviewers changed from JeanPhilippe Labbé to JeanPhilippe Labbé, Travis Scrimshaw
 Status changed from needs_review to needs_work
I have a few comments:
 def matrix_space(self, nrows=None, ncols=None, sparse=None): + def matrix_space(self, nrows=None, ncols=None, sparse=True): r""" Return a matrix space over the integers. INPUT:   ``nrows``  number of rows +  ``nrows``  number of rows   ``ncols``  number of columns +  ``ncols``  number of columns   ``sparse``  (boolean) sparseness +  ``sparse``  (boolean) sparseness EXAMPLES:: sage: cm = CartanMatrix(['A', 3]) sage: cm.matrix_space() Full MatrixSpace of 3 by 3 sparse matrices over Integer Ring sage: cm.matrix_space(2, 2) Full MatrixSpace of 2 by 2 sparse matrices over Integer Ring sage: cm[:2,1:] # indirect doctest [1 0] [ 2 1] """ if nrows is None: nrows = self.nrows() if ncols is None: ncols = self.ncols()  if sparse is None:  sparse = True if nrows == self.nrows() and ncols == self.ncols() and sparse: return self.parent() else: from sage.matrix.matrix_space import MatrixSpace return MatrixSpace(ZZ, nrows, ncols, sparse is None or bool(sparse))
I would make this into a more 2/3 compatible test:
sage: for i in range(3): ....: for j in range(3): ....: try: ....: s = M[i].an_element() * M[j].an_element() ....: print('X', end='') ....: except TypeError: ....: print(' ', end='') ....: print()
I would instead create a 0/1matrix and output that. Another option would be creating an appropriate string with '\n'.join(...)
. Similar for another test.
These tests should not be marked as meataxe if it really is using the generic implementation:
sage: MS = MatrixSpace(M.base_ring(), M.nrows(), M.ncols(), implementation='generic') sage: X = MS(M) # optional: meataxe sage: type(X) # optional: meataxe sage: X.echelon_form() # optional: meataxe
Otherwise that would be a bug.
Do we want m4ri to have precedence over trying meataxe when no implementation is given? (I'm ccing Simon to note that there might be a change in behavior.)
I'm also split on documenting about when we use a nongeneric implementation by default. It feels too much like an implementation detail that it should be hidden from the more casual users, but I think it is good for the more advanced users to have it in the public doc. Thoughts?
I'm also not 100% convinced about having to explicitly convert matrices of different implementations, but since things are likely to be incompatible and it seems unlikely that this would be a problem (I think a user who is choosing implementations will probably be careful and might even appreciate this error). I do not have a good reason to make this an objection, but I am just noting that this might come up from a user.
comment:57 Changed 2 years ago by
 Commit changed from 4f818deff90d1db47f55603509d22f0c267fcf8a to eeeba324b329c69b8e7c625423cfe4eca4862fc2
Branch pushed to git repo; I updated commit sha1. New commits:
eeeba32  23706: a default argument and doctests

comment:58 in reply to: ↑ 56 ; followup: ↓ 60 Changed 2 years ago by
Thanks Travis for having a look.
Replying to tscrim:
[SNIP]
agreed and changed in eeeba32
Do we want m4ri to have precedence over trying meataxe when no implementation is given? (I'm ccing Simon to note that there might be a change in behavior.)
There should not be any change. It concerns only the case characteristic = 2
.
if implementation is None: if R.characteristic() == 2 and R.order() <= 65536: implementation = 'm4ri' elif R.order() <= 255: try: from . import matrix_gfpn_dense except ImportError: implementation = 'generic' else: implementation = 'meataxe' else: implementation = 'generic'
where it used to be
if R.characteristic() == 2: if R.order() <= 65536: return matrix_gf2e_dense.Matrix_gf2e_dense elif R.order() <= 255: try: from . import matrix_gfpn_dense return matrix_gfpn_dense.Matrix_gfpn_dense except ImportError: pass
I'm also split on documenting about when we use a nongeneric implementation by default. It feels too much like an implementation detail that it should be hidden from the more casual users, but I think it is good for the more advanced users to have it in the public doc. Thoughts?
What would you expect exactly? It would be a nightmare to keep the documentation up to date.
I'm also not 100% convinced about having to explicitly convert matrices of different implementations, but since things are likely to be incompatible and it seems unlikely that this would be a problem (I think a user who is choosing implementations will probably be careful and might even appreciate this error). I do not have a good reason to make this an objection, but I am just noting that this might come up from a user.
It should be possible to implement coercion to the default implementation (with conversion). Anyway, I would like to keep such changes for later.
comment:59 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:60 in reply to: ↑ 58 Changed 2 years ago by
 Status changed from needs_review to positive_review
Replying to vdelecroix:
Thanks Travis for having a look.
Do we want m4ri to have precedence over trying meataxe when no implementation is given? (I'm ccing Simon to note that there might be a change in behavior.)
There should not be any change. It concerns only the case
characteristic = 2
. [snip]
Right, actually I might have misread what meataxe is for and it's a nonissue because it is not meant for characteristic 2. Sorry for the noise.
I'm also split on documenting about when we use a nongeneric implementation by default. It feels too much like an implementation detail that it should be hidden from the more casual users, but I think it is good for the more advanced users to have it in the public doc. Thoughts?
What would you expect exactly? It would be a nightmare to keep the documentation up to date.
Keeping it up to date would not be such a problem; at least I don't expect it to change that much. However, I tried a few different ways to present the info and nothing really worked too well. There's just a few too many little cases for the finite fields. I guess if someone cares enough they are good enough to just look at the code.
Thanks. Positive review.
comment:61 Changed 2 years ago by
 Status changed from positive_review to needs_work
Documentation doesn't build
comment:62 Changed 2 years ago by
 Commit changed from eeeba324b329c69b8e7c625423cfe4eca4862fc2 to 6bff8c84a2cf7013f1bbc1b1a1cf737454a6a888
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c06ed6e  23706: allow to choose matrix implementation

758947e  23706: fix for CartanMatrix

e394c15  Some pep8 conventions

c16fc79  23706: fix doctest in matrix_gfpn_dense.pyx

6bff8c8  23706: a default argument and doctests

comment:63 Changed 2 years ago by
 Milestone changed from sage8.1 to sage8.2
 Status changed from needs_work to needs_review
With the rebasing I do not have any problem with the documentation.
comment:64 Changed 2 years ago by
 Status changed from needs_review to needs_work
found out
[dochtml] [matrices ] docstring of sage.matrix.action.MatrixMatrixAction:24: WARNING: Unexpected indentation. [dochtml] [matrices ] docstring of sage.matrix.action.MatrixMatrixAction:25: WARNING: Block quote ends without a blank line; unexpected unindent. [dochtml] [matrices ] /opt/sage/local/lib/python2.7/sitepackages/sage/matrix/special.py:docstring of sage.matrix.special:48: WARNING: undefined label: sage.combinat.matrices.hadamard_matrix (if the link has no caption the label must precede a section header) [dochtml] [matrices ] /opt/sage/local/lib/python2.7/sitepackages/sage/matrix/special.py:docstring of sage.matrix.special:49: WARNING: undefined label: sage.combinat.matrices.latin (if the link has no caption the label must precede a section header) [dochtml] Error building the documentation. [dochtml] Traceback (most recent call last): [dochtml] File "/opt/sage/local/lib/python2.7/runpy.py", line 174, in _run_module_as_main [dochtml] "__main__", fname, loader, pkg_name) [dochtml] File "/opt/sage/local/lib/python2.7/runpy.py", line 72, in _run_code [dochtml] exec code in run_globals [dochtml] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__main__.py", line 2, in <module> [dochtml] main() [dochtml] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 1675, in main [dochtml] builder() [dochtml] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 310, in _wrapper [dochtml] getattr(get_builder(document), 'inventory')(*args, **kwds) [dochtml] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 505, in _wrapper [dochtml] build_many(build_ref_doc, L) [dochtml] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 260, in build_many [dochtml] results.append(target(arg)) [dochtml] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 71, in build_ref_doc [dochtml] getattr(ReferenceSubBuilder(doc, lang), format)(*args, **kwds) [dochtml] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 720, in _wrapper [dochtml] getattr(DocBuilder, build_type)(self, *args, **kwds) [dochtml] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 105, in f [dochtml] runsphinx() [dochtml] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/sphinxbuild.py", line 218, in runsphinx [dochtml] raise exception [dochtml] OSError: [matrices ] docstring of sage.matrix.action.MatrixMatrixAction:24: WARNING: Unexpected indentation. [dochtml] [dochtml] [matrices ] WARNING: Error building the documentation. [dochtml] [matrices ] Traceback (most recent call last): [dochtml] [matrices ] File "/opt/sage/local/lib/python2.7/runpy.py", line 174, in _run_module_as_main [dochtml] [matrices ] "__main__", fname, loader, pkg_name) [dochtml] [matrices ] File "/opt/sage/local/lib/python2.7/runpy.py", line 72, in _run_code [dochtml] [matrices ] exec code in run_globals [dochtml] [matrices ] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__main__.py", line 2, in <module> [dochtml] [matrices ] main() [dochtml] [matrices ] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 1675, in main [dochtml] [matrices ] builder() [dochtml] [matrices ] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 310, in _wrapper [dochtml] [matrices ] getattr(get_builder(document), 'inventory')(*args, **kwds) [dochtml] [matrices ] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 505, in _wrapper [dochtml] [matrices ] build_many(build_ref_doc, L) [dochtml] [matrices ] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 260, in build_many [dochtml] [matrices ] results.append(target(arg)) [dochtml] [matrices ] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 71, in build_ref_doc [dochtml] [matrices ] getattr(ReferenceSubBuilder(doc, lang), format)(*args, **kwds) [dochtml] [matrices ] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 720, in _wrapper [dochtml] [matrices ] getattr(DocBuilder, build_type)(self, *args, **kwds) [dochtml] [matrices ] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 105, in f [dochtml] [matrices ] runsphinx() [dochtml] [matrices ] File "/opt/sage/local/lib/python2.7/sitepackages/sage_setup/docbuild/sphinxbuild.py", line 218, in runsphinx [dochtml] [matrices ] raise exception [dochtml] [matrices ] OSError: [matrices ] docstring of sage.matrix.action.MatrixMatrixAction:24: WARNING: Unexpected indentation.
comment:65 Changed 2 years ago by
 Commit changed from 6bff8c84a2cf7013f1bbc1b1a1cf737454a6a888 to 018226b9ecafdc082a4eb8289269af85b2908d3b
Branch pushed to git repo; I updated commit sha1. New commits:
018226b  23706: separate the NOTE and TESTS blocks

comment:66 Changed 2 years ago by
 Status changed from needs_work to needs_review
I don't understand why, but sphinx was in trouble with NOTE
and TESTS
being in the same documentation string.
comment:67 Changed 2 years ago by
It may not be Sphinx but our custom handling of the TESTS
block (at least, IIRC we do something special there in order to hide the block).
comment:68 Changed 2 years ago by
 Status changed from needs_review to positive_review
Anyways, the workaround is fine with me and there is a green patchbot. Back to positive review.
comment:69 Changed 2 years ago by
thanks Travis.
comment:70 Changed 2 years ago by
 Branch changed from u/vdelecroix/23706 to 018226b9ecafdc082a4eb8289269af85b2908d3b
 Resolution set to fixed
 Status changed from positive_review to closed
experimental branch ready!
New commits:
23704: getitem/setitem for libgap elements
TMP 23706 commit