netwulf icon indicating copy to clipboard operation
netwulf copied to clipboard

Adding changes in CHANGELOG.md

Open benmaier opened this issue 5 years ago • 3 comments

I've noticed that for v0.2 several changes were made that have not been logged in CHANGELOG.md. So far I've found:

  • the node attribute radius is now called size. Ulf, could you explain why you chose to rename this? I like radius because I know what it means because it has a geometric defnition. size is rather ambiguous.
  • when redrawing with matplotlib, overlapping nodes are joined such that strokes around nodes do not show anymore. was that intended? I think it makes the visualizations kind of awkward looking.

benmaier avatar Sep 24 '20 10:09 benmaier

the node attribute radius is now called size. Ulf, could you explain why you chose to rename this? I like radius because I know what it means because it has a geometric defnition. size is rather ambiguous.

Good question. In v0.1 we ambiguously use size and radius, and I think I just wanted to clear this up by using simply one of them. But I agree that radius is better.

when redrawing with matplotlib, overlapping nodes are joined such that strokes around nodes do not show anymore. was that intended? I think it makes the visualizations kind of awkward looking.

I fixed this in the latest commit 7964e1a3324c7227f69db45124cacd78e4af2c9a. But I have a question: In v0.1, matplotlib rendering is simple because it just involves passing one collection of ellipses to the axis object, like so:

        # v0.1: tools.py, line 429
        circles = EllipseCollection(size,size,np.zeros_like(size),
                                    offsets=XY,
                                    units='x',
                                    transOffset=ax.transData,
                                    facecolors=node_colors,
                                    linewidths=network_properties['nodeStrokeWidth']/width*axwidth,
                                    edgecolors=network_properties['nodeStrokeColor'],
                                    zorder=zorder
                                )
        ax.add_collection(circles)

When I reused this code in v0.2, node edges became too wide, compared to those rendered in JS. This, I found, is because node edges are rendered on top of node fill in matplotlib, but behind in JS. The fix for this (in the latest commit) is to add nodes and node edges as separate objects, one at a time.

        # v0.2: tools.py, line 423
        for xy, r, c in zip(XY, size, node_colors):
            circle = mpl.patches.Ellipse(xy, r, r, facecolor=c)
            stroke = mpl.patches.Ellipse(
                xy, r, r, facecolor=None,
                edgecolor=network_properties['nodeStrokeColor'],
                linewidth=network_properties['nodeStrokeWidth'] / width * axwidth
            )
            ax.add_artist(stroke)
            ax.add_artist(circle)

This is obviously slower (but not devastatingly slow). What's your opinion on the fix? I agree that it's hacky, but I couldn't find another way to render node edges behind (or in front in JS).

ulfaslak avatar Sep 24 '20 11:09 ulfaslak

ah, cool! thanks for the quick reply.

Yeah, I remember that I encountered the same problem with edge widths once and I also remember that I had an approximate fix by scaling the edge widths, which doesn't really solve the problem.

I think adding nodes and edges one add a time was realllly slow back then for me which is why decided to live with the edge width problem. I'm not sure how much better matplotlib got at this by now.

benmaier avatar Sep 24 '20 12:09 benmaier

We could make it optional? Most users won't care.

So default is hack-scaling (as in v0.1, most users won't notice) and optionally you can do precise but slow rendering, one node at a time.

ulfaslak avatar Sep 25 '20 09:09 ulfaslak