paulbrodersen / netgraph

Publication-quality network visualisations in python
GNU General Public License v3.0
667 stars 40 forks source link

Issues with MultiGraph #76

Closed dg-pb closed 11 months ago

dg-pb commented 12 months ago

Hello again, so there are several points, that I have encountered.

  1. Just to repeat it so it doesn't get lost. Same bug that I have indicated before can be found here: https://github.com/paulbrodersen/netgraph/blob/dev/netgraph/_interactive_variants.py#L130
  2. node_label are being "fitted" into node, even if node_label_offset is not == (0, 0). Long names are just impossible to see given text is being rescaled with no possibility to zoom in.
  3. There are couple of places, where nodes and edges are being sorted. This raises an error if nodes and labels are not of the same type. I used key argument for sorted to solve the issue at first, then I duplicated my graph before plotting converting nodes to same type.

Thank you.

paulbrodersen commented 11 months ago

I have a bad cold at the moment, so my brain is at 20% efficiency right now but I will look into these issues once I am better, hopefully later this week.

node_label are being "fitted" into node, even if node_label_offset is not == (0, 0). Long names are just impossible to see given text is being rescaled with no possibility to zoom in.

Shouldn't happen and I will fix that but in the meantime, setting the font size explicitly with node_label_fontdict=dict(size=12) (or similar) should fix the font size to -- in this case -- 12 pts.

There are couple of places, where nodes and edges are being sorted. This raises an error if nodes and labels are not of the same type. I used key argument for sorted to solve the issue at first, then I duplicated my graph before plotting converting nodes to same type.

Can you post the error trace? Don't really see where or why that would happen.

dg-pb commented 11 months ago

These two were the issues in my case: netgraph/_utils.py: 932 netgraph/_edge_layout.py: 1314

But check the others too:

# grep sorted
./_node_layout.py:    nodes = sorted(_get_unique_nodes(edges))
./_node_layout.py:    nodes = sorted(_get_unique_nodes(edges))
./_node_layout.py:    return left, sorted(right_ranks, key=right_ranks.get)
./_utils.py:    components = sorted(components, key=len, reverse=True)
./_utils.py:    # return sorted(_map_multigraph_source_target_pairs_to_edge_ids(edges))

Hope you get well soon.

paulbrodersen commented 11 months ago

I can't reproduce the second issue. Node labels are not rescaled for non-zero node label offsets.

from netgraph import Graph
Graph([(0, 1)], node_labels={0 : "lorem ipsum", 1 : "dolor sit"}, node_label_offset=0.1)
plt.show()

Can you provide a MWE that produces the issue on your end?

dg-pb commented 11 months ago

It seems that the issue is specific to EditableMultiGraph (Maybe EditableGraph too).

from netgraph import EditableMultiGraph
g = EditableMultiGraph([(0, 1, 0)], node_labels={0 : "lorem ipsum", 1 : "dolor sit"}, node_label_offset=0.1, node_label_fontdict=dict(size=40))
plt.show()

Try Editable and non-editable and see the difference. I am not sure if the issue is exactly what I have indicated, but the font size is small and I could not find a way to make it bigger.

And I do need editable, cause there is a lot of label overlap in initial initial positioning of complex graphs. Can't see much from it..

dg-pb commented 11 months ago

What about 3? Is that sorting needed?

paulbrodersen commented 11 months ago

Most (if not all) sorts are only there to make badly written tests reproducible. So, no, probably not, but I am slightly reluctant to remove the sorting while my brain is still this sluggish.

paulbrodersen commented 11 months ago

Mixed node types are now supported in Netgraph. I either removed calls to sorted or wrapped them in a try/except block.

paulbrodersen commented 11 months ago

I will close the issue for now as I think I have addressed all sub-issues but feel free to re-open if that turns out not to be the case.

dg-pb commented 11 months ago

Thank you! One more error:

_utils.py:120

File "/usr/local/lib/python3.11/site-packages/netgraph/_utils.py", line 120, in _edge_list_to_adjacency_list
    edges = edges + [(target, source) for (source, target) in edges] # forces copy
            ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
TypeError: unsupported operand type(s) for +: 'set' and 'list'

if I wrap edges in list:

if not directed:
        edges = list(edges) + [(target, source) for (source, target) in edges] # forces copy

All works fine, but I wander if it was ever meant to be set in the first place?

it gets converted to a set in _utils.py:934:

def _simplify_multigraph(edges):
    return {(source, target) for source, target, _ in edges}

The other concern is that I am running MultiDiGraph, so that if not directed should not be evaluated to True in the first place.

dg-pb commented 11 months ago

So solution to solve this could be:

def _simplify_multigraph(edges):
    return list({(source, target) for source, target, _ in edges})

But also, have a look at that argument, which is hardcoded at: netgraph/_node_layout.py:53

adjacency_list = _edge_list_to_adjacency_list(edges, directed=False)

although it is being used for directed graphs...

dg-pb commented 11 months ago

All works now. Thank you.

paulbrodersen commented 11 months ago

So solution to solve this could be:

def _simplify_multigraph(edges):
    return list({(source, target) for source, target, _ in edges})

There used to be a sorted call there that returned a list instead of a set. ;-) But thanks for the heads up; should be fixed now.

The hard-coded directed=False is correct though. The Fruchterman-Reingold algorithm (as pretty much all node layout algorithms) was developed for undirected graphs. If you use the directed graph instead, the algorithm typically requires a lot more iterations to settle on solution, which is typically worse (longer average edge lengths, more edge crossings).

paulbrodersen commented 11 months ago

And: one does get notifications on closed issues.

dg-pb commented 11 months ago

Ok, thank you. Good to know.