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

Adding changes in CHANGELOG.md #46

Open
benmaier opened this issue Sep 24, 2020 · 3 comments
Open

Adding changes in CHANGELOG.md #46

benmaier opened this issue Sep 24, 2020 · 3 comments
Assignees

Comments

@benmaier
Copy link
Owner

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.
@ulfaslak
Copy link
Collaborator

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 7964e1a. 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).

@benmaier
Copy link
Owner Author

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.

@ulfaslak
Copy link
Collaborator

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.

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

No branches or pull requests

2 participants