Opened 5 years ago
Closed 4 years ago
#23706 closed enhancement (fixed)
allow several implementations of matrices in MatrixSpace
Reported by: | vdelecroix | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.2 |
Component: | linear algebra | Keywords: | days88 |
Cc: | jipilab, SimonKing | Merged in: | |
Authors: | Vincent Delecroix | Reviewers: | Jean-Philippe Labbé, Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | 018226b (Commits, GitHub, GitLab) | 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 5 years ago by
- Description modified (diff)
comment:2 Changed 5 years ago by
- Keywords sd88 added
comment:3 Changed 5 years ago by
- Keywords days88 added; sd88 removed
comment:4 Changed 5 years ago by
- Branch set to u/vdelecroix/23706
- Commit set to 34f1ddc7e0ea69ab280567bf68da69d5fb2f0cd2
comment:5 Changed 5 years ago by
- Reviewers set to Jean-Philippe Labbé
The title of the ticket could be a bit more specific.
comment:6 Changed 5 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 5 years ago by
- Commit changed from d9f0532af4ca79ba467e466c741416b4409ca960 to 007bdcf7710533d0fd413c56d9d566b84db1873f
comment:8 Changed 5 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 5 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 5 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 5 years ago by
- Description modified (diff)
- Summary changed from gap class for matrices to allow several implementations of matrices in MatrixSpace
comment:12 Changed 5 years ago by
- Description modified (diff)
comment:13 Changed 5 years ago by
- Commit changed from dce22c869eee2d2b4a6f59cd9f2e6b5eda633121 to f1013de2bc8d01830840422c367dbe9224d0aeb6
comment:14 Changed 5 years ago by
- Dependencies set to #23704
- Status changed from new to needs_review
comment:15 Changed 5 years ago by
- Description modified (diff)
comment:16 Changed 5 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 5 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 5 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 5 years ago by
- Commit changed from 0cbf7ff5a1e4ec51bfd4251e7a0677b77ec945ce to 9f74040baa887b1ff6e286cbccb7af77f364c6f3
comment:20 Changed 5 years ago by
- Dependencies #23704, #23721 deleted
- Description modified (diff)
comment:21 Changed 5 years ago by
- Status changed from needs_work to needs_review
I postponed the introduction of Matrix_gap
in ticket #23714
comment:23 Changed 5 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 5 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
comment:25 Changed 5 years ago by
JP, do you feel up for the review?
comment:26 Changed 5 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 5 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 5 years ago by
- Description modified (diff)
comment:29 Changed 5 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 5 years ago by
Cartan matrices are fixed... waiting for patchbot verdict again.
comment:31 Changed 5 years ago by
- Description modified (diff)
comment:32 follow-up: ↓ 40 Changed 5 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 5 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 follow-up: ↓ 37 Changed 5 years ago by
I ran the test on the file src/sage/matrix/matrix_gfpn_dense.pyx
and all tests passed...
comment:35 Changed 5 years ago by
- Branch changed from u/vdelecroix/23706 to u/jipilab/23706
comment:36 Changed 5 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 5 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 5 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 5 years ago by
Much cleaner now!!
comment:40 in reply to: ↑ 32 Changed 5 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 follow-up 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 follow-up: ↓ 44 Changed 5 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 follow-up: ↓ 43 Changed 5 years ago by
Is there a dependency to #23721?
comment:43 in reply to: ↑ 42 Changed 5 years ago by
comment:44 in reply to: ↑ 41 ; follow-up: ↓ 46 Changed 5 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 5 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 5 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 5 years ago by
It's ok.
comment:48 Changed 5 years ago by
There are some spurious doc errors, otherwise, it looks good.
comment:49 Changed 5 years ago by
- Commit changed from 869b155d053a8f4cb8d69d2575328b2d3ceabc86 to 079981ab5ca646a65072815b7b956060b908f5f7
comment:50 Changed 5 years ago by
j'ai poussé... reste plus qu'à attendre les verdits de madame patchbot.
comment:51 Changed 5 years ago by
- Branch changed from public/23706 to u/vdelecroix/23706
- Commit changed from 079981ab5ca646a65072815b7b956060b908f5f7 to 5c415f5070e1e4cfb0e57c3c99cdd1470114e2ed
comment:53 Changed 5 years ago by
- Commit changed from 5c415f5070e1e4cfb0e57c3c99cdd1470114e2ed to 4f818deff90d1db47f55603509d22f0c267fcf8a
comment:54 Changed 5 years ago by
rebased on 8.1.rc0
comment:55 Changed 5 years ago by
patchbot is happy! jipilab?
comment:56 follow-up: ↓ 58 Changed 5 years ago by
- Cc SimonKing added
- Reviewers changed from Jean-Philippe Labbé to Jean-Philippe 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/1-matrix 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 cc-ing Simon to note that there might be a change in behavior.)
I'm also split on documenting about when we use a non-generic 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 5 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 ; follow-up: ↓ 60 Changed 5 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 cc-ing 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 non-generic 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 5 years ago by
- Status changed from needs_work to needs_review
comment:60 in reply to: ↑ 58 Changed 5 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 cc-ing 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 non-issue because it is not meant for characteristic 2. Sorry for the noise.
I'm also split on documenting about when we use a non-generic 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 4 years ago by
- Status changed from positive_review to needs_work
Documentation doesn't build
comment:62 Changed 4 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 4 years ago by
- Milestone changed from sage-8.1 to sage-8.2
- Status changed from needs_work to needs_review
With the rebasing I do not have any problem with the documentation.
comment:64 Changed 4 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/site-packages/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/site-packages/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/site-packages/sage_setup/docbuild/__main__.py", line 2, in <module> [dochtml] main() [dochtml] File "/opt/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 1675, in main [dochtml] builder() [dochtml] File "/opt/sage/local/lib/python2.7/site-packages/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/site-packages/sage_setup/docbuild/__init__.py", line 505, in _wrapper [dochtml] build_many(build_ref_doc, L) [dochtml] File "/opt/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 260, in build_many [dochtml] results.append(target(arg)) [dochtml] File "/opt/sage/local/lib/python2.7/site-packages/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/site-packages/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/site-packages/sage_setup/docbuild/__init__.py", line 105, in f [dochtml] runsphinx() [dochtml] File "/opt/sage/local/lib/python2.7/site-packages/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/site-packages/sage_setup/docbuild/__main__.py", line 2, in <module> [dochtml] [matrices ] main() [dochtml] [matrices ] File "/opt/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 1675, in main [dochtml] [matrices ] builder() [dochtml] [matrices ] File "/opt/sage/local/lib/python2.7/site-packages/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/site-packages/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/site-packages/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/site-packages/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/site-packages/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/site-packages/sage_setup/docbuild/__init__.py", line 105, in f [dochtml] [matrices ] runsphinx() [dochtml] [matrices ] File "/opt/sage/local/lib/python2.7/site-packages/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 4 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 4 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 4 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 4 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 4 years ago by
thanks Travis.
comment:70 Changed 4 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