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.

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

Incidence matrix method doesnt work for G = DiGraph({0: ['a']}) #29277

Open
vipul79321 mannequin opened this issue Mar 3, 2020 · 6 comments
Open

Incidence matrix method doesnt work for G = DiGraph({0: ['a']}) #29277

vipul79321 mannequin opened this issue Mar 3, 2020 · 6 comments

Comments

@vipul79321
Copy link
Mannequin

vipul79321 mannequin commented Mar 3, 2020

Current behavior:

sage: G = DiGraph({0: ['a']})
sage: G.incidence_matrix()
Traceback (most recent call last)
...
TypeError: '<' not supported between instances of 'int' and 'str'

incidence_matrix method should be able to handle the case when vertices are of mismatched types.

Depends on #22349

CC: @dcoudert

Component: graph theory

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

@vipul79321 vipul79321 mannequin added this to the sage-9.1 milestone Mar 3, 2020
@dcoudert
Copy link
Contributor

dcoudert commented Mar 4, 2020

comment:1

When vertices are of different types, a solution is to give as input an ordering of the vertices

sage: G = DiGraph({0: ['a']})
sage: G.incidence_matrix(vertices=[0, 'a'])
[-1]
[ 1]
sage: G.incidence_matrix(vertices=list(G))
[ 1]
[-1]

and we will have to proceed with #22349 to stop sorting vertices and edges by default (not easy).

@vipul79321
Copy link
Mannequin Author

vipul79321 mannequin commented Mar 7, 2020

comment:2

Replying to @dcoudert:

When vertices are of different types, a solution is to give as input an ordering of the vertices

sage: G = DiGraph({0: ['a']})
sage: G.incidence_matrix(vertices=[0, 'a'])
[-1]
[ 1]
sage: G.incidence_matrix(vertices=list(G))
[ 1]
[-1]

and we will have to proceed with #22349 to stop sorting vertices and edges by default (not easy).

I was wondering why can't we make the following changes:
Instead of calling self.vertices(), we can call self.vertices(sort=False) which will solve our problem. And then we can modify doc-tests and documentation accordingly.

Also, We are kind of expecting the same behavior as above from fixing of #22349

@dcoudert
Copy link
Contributor

dcoudert commented Mar 8, 2020

comment:3

We did a hard work on the graph module to make it compatible with Python3. We let this case for compatibility with other codes for the moment. The compromise was to introduce parameter vertices.
Also, I prefer to first deprecate default sorting of vertices and edges before changing the behavior of this method. Meanwhile, we can document the issue in the doctests of the method to should that it is better to specify the order of vertices and edges.

@mkoeppe
Copy link
Contributor

mkoeppe commented May 1, 2020

comment:4

Moving tickets to milestone sage-9.2 based on a review of last modification date, branch status, and severity.

@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 May 1, 2020
@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Oct 24, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 7, 2021

comment:6

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Apr 7, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Aug 22, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 8, 2022

Dependencies: #22349

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants