Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BipartiteGraph.reduced_adjacency_matrix: accept keyword arguments for matrix constructor #33387

Closed
dcoudert opened this issue Feb 20, 2022 · 16 comments
Closed

BipartiteGraph.reduced_adjacency_matrix: accept keyword arguments for matrix constructor #33387

dcoudert opened this issue Feb 20, 2022 · 16 comments
Labels
c: graph theory p: major / 3 t: enhancement
Milestone
sage-9.6

Comments

@dcoudert
Copy link
Contributor

Following #33377, we add arguments to method BipartiteGraph.reduced_adjacency_matrix for the matrix constructor.

Depends on #33377

CC: @mkoeppe

Component: graph theory

Author: David Coudert

Branch/Commit: 0abc581

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/33387
@dcoudert dcoudert added this to the sage-9.6 milestone Feb 20, 2022
@dcoudert
Copy link
Contributor Author

Last 10 new commits:

fe1da0aMerge #32465
007253dsrc/sage/matrix/matrix_space.py (get_matrix_class): Handle base_ring=ZZ, implementation='numpy'
01d5541GenericGraph.adjacency_matrix: Accept keyword arguments for matrix constructor
03343f7GenericGraph.adjacency_matrix: Add doctest with immutable=True
688d054GenericGraph.weighted_adjacency_matrix: Accept keyword arguments for matrix constructor
e30c7cbGenericGraph.weighted_adjacency_matrix: Make base_ring a keyword-only argument
6f2185aGenericGraph.incidence_matrix: Accept keyword arguments for matrix constructor
0bd2930GenericGraph._matrix_: Use the new keyword argument base_ring of GenericGraph.adjacency_matrix
fc8514ctrac #33387: BipartiteGraph.reduced_adjacency_matrix: accept keyword arguments for matrix constructor
23f79dctrac #33387: pass **kwds to matrix constructor

@dcoudert
Copy link
Contributor Author

Commit: 23f79dc

@dcoudert
Copy link
Contributor Author

Branch: public/graphs/33387

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 20, 2022

comment:2

typo: otherwisse -> otherwise

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 20, 2022

comment:3

Otherwise, LGTM, feel free to set to positive review after the fix

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 20, 2022

Reviewer: Matthias Koeppe

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 20, 2022

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

3c12fc9trac #33387: fix typo

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 20, 2022

Changed commit from 23f79dc to 3c12fc9

@dcoudert
Copy link
Contributor Author

comment:5

Thanks for the review.

@kiwifb
Copy link
Member

kiwifb commented Feb 23, 2022

comment:6

I get

sage -t --long --random-seed=234917630320381229401386766811170895410 /usr/lib/python3.10/site-packages/sage/graphs/bipartite_graph.py
**********************************************************************
File "/usr/lib/python3.10/site-packages/sage/graphs/bipartite_graph.py", line 1678, in sage.graphs.bipartite_graph.BipartiteGraph.reduced_adjacency_matrix
Failed example:
    B.reduced_adjacency_matrix(base_ring=RDF)
Expected:
    Traceback (most recent call last):
    ...
    TypeError: float() argument must be a string or a number, not 'sage.rings.finite_rings.element_givaro.FiniteField_givaroElement'
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/usr/lib/python3.10/site-packages/sage/doctest/forker.py", line 694, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3.10/site-packages/sage/doctest/forker.py", line 1088, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.graphs.bipartite_graph.BipartiteGraph.reduced_adjacency_matrix[35]>", line 1, in <module>
        B.reduced_adjacency_matrix(base_ring=RDF)
      File "/usr/lib/python3.10/site-packages/sage/graphs/bipartite_graph.py", line 1718, in reduced_adjacency_matrix
        return matrix(base_ring, len(self.right), len(self.left), D, sparse=sparse, **kwds)
      File "sage/matrix/constructor.pyx", line 643, in sage.matrix.constructor.matrix (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_10/cythonized/sage/matrix/constructor.c:2725)
        M = MatrixArgs(*args, **kwds).matrix()
      File "sage/matrix/args.pyx", line 667, in sage.matrix.args.MatrixArgs.matrix (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_10/cythonized/sage/matrix/args.c:8006)
        M = self.space(self, coerce=convert)
      File "sage/structure/parent.pyx", line 899, in sage.structure.parent.Parent.__call__ (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_10/cythonized/sage/structure/parent.c:9471)
        return mor._call_with_args(x, args, kwds)
      File "sage/structure/coerce_maps.pyx", line 180, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_with_args (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_10/cythonized/sage/structure/coerce_maps.c:5259)
        raise
      File "sage/structure/coerce_maps.pyx", line 170, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_with_args (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_10/cythonized/sage/structure/coerce_maps.c:5049)
        return C._element_constructor(x, **kwds)
      File "/usr/lib/python3.10/site-packages/sage/matrix/matrix_space.py", line 928, in _element_constructor_
        return self.element_class(self, entries, **kwds)
      File "sage/matrix/matrix_generic_sparse.pyx", line 170, in sage.matrix.matrix_generic_sparse.Matrix_generic_sparse.__init__ (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_10/cythonized/sage/matrix/matrix_generic_sparse.c:3332)
        self._entries = ma.dict(coerce)
      File "sage/matrix/args.pyx", line 770, in sage.matrix.args.MatrixArgs.dict (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_10/cythonized/sage/matrix/args.c:8863)
        for t in self.iter(convert, True):
      File "sage/matrix/args.pyx", line 544, in iter (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_10/cythonized/sage/matrix/args.c:7026)
        se = make_SparseEntry(se.i, se.j, self.base(se.entry))
      File "sage/structure/parent.pyx", line 897, in sage.structure.parent.Parent.__call__ (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_10/cythonized/sage/structure/parent.c:9444)
        return mor._call_(x)
      File "sage/structure/coerce_maps.pyx", line 161, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_10/cythonized/sage/structure/coerce_maps.c:4728)
        raise
      File "sage/structure/coerce_maps.pyx", line 156, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_10/cythonized/sage/structure/coerce_maps.c:4620)
        return C._element_constructor(x)
      File "sage/rings/real_double.pyx", line 740, in sage.rings.real_double.RealDoubleElement.__init__ (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_10/cythonized/sage/rings/real_double.c:9480)
        self._value = float(x)
    TypeError: float() argument must be a string or a real number, not 'sage.rings.finite_rings.element_givaro.FiniteField_givaroElement'

The key bits are

TypeError: float() argument must be a string or a number, not 'sage.rings.finite_rings.element_givaro.FiniteField_givaroElement'

is expected but

TypeError: float() argument must be a string or a real number, not 'sage.rings.finite_rings.element_givaro.FiniteField_givaroElement'

is returned. This is with python 3.10 and cython 0.29.26 if it is important.

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 23, 2022

comment:8

Some ... is needed to match the refined error messages in Python 3.10

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 24, 2022

Changed commit from 3c12fc9 to 0abc581

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 24, 2022

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

0abc581trac #33387: fix test with Python 3.10

@dcoudert
Copy link
Contributor Author

comment:10

Let me know if this is ok or if I should shrink the line with the error message.

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 24, 2022

comment:11

This should work, thanks

@vbraun
Copy link
Member

vbraun commented Feb 27, 2022

Changed branch from public/graphs/33387 to 0abc581

@vbraun vbraun closed this as completed in 4c814d6 Feb 27, 2022
@rlmill rlmill mannequin mentioned this issue Jan 29, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: graph theory p: major / 3 t: enhancement
Projects
None yet
Milestone
sage-9.6
Development

No branches or pull requests
4 participants
@kiwifb @vbraun @mkoeppe @dcoudert