Becksteinlab / kda

Python package used for the analysis of biochemical kinetic diagrams.
GNU General Public License v3.0
3 stars 1 forks source link

BUG, MAINT: Fix `arrows=True` parameter for `nx.draw_networkx_edges` in `plotting.draw_diagrams()` #80

Closed nawtrey closed 5 months ago

nawtrey commented 11 months ago

Issue description

Generating partial diagrams with using plotting.draw_diagrams() with arrows=False results in the following error:

The arrowstyle keyword argument is not applicable when drawing edges
with LineCollection.

To make this warning go away, either specify `arrows=True` to
force FancyArrowPatches or use the default value for arrowstyle.
Note that using FancyArrowPatches may be slow for large graphs.

  nx.draw_networkx_edges(

This is because we are specifying an arrowstyle for an undirected diagram. Previously this parameter was ignored but the newer versions of networkx are suggesting we only provide the parameter for undirected graphs.

Proposed Solution

An easy solution would be to detect the type of graph and vary our input parameters based on that (i.e. only use arrows=True and arrowstyle when the input graph is directed).

Comments

Not a pressing issue as it only raises a UserWarning, but #79 attempted a naïve solution and it needs to be repaired. Simply adding arrows=True for all cases does not work because partial diagrams are generated with arrows when they shouldn't be represented as directed.

nawtrey commented 11 months ago

According to the source code, the warning will be raised if any of the following parameters are not met when arrows=False:

    if use_linecollection and any(
        [
            arrowstyle is not None,
            arrowsize != 10,
            connectionstyle != "arc3",
            min_source_margin != 0,
            min_target_margin != 0,
        ]

so the solution will have to set all of these back to the default for undirected graphs (i.e. partial diagrams).

Assuming we only use nx.is_directed() as the switch for handling these cases, that means that we will lose the functionality to generate undirected variants of directed graphs. This is probably "okay" for now since we generally draw undirected graphs with fancy arrows since they are a more accurate representation of the graph. If this is desired for something like a flux diagram, we may have to find a way to specify this manually, possibly by adding a new parameter to draw_diagrams(), but again this is not something in common use.