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

Fix setting edge colors for Chord graphs using an explicit mapping #3734

Merged
merged 1 commit into from Jul 26, 2019

Conversation

@schumann-tim
Copy link
Contributor

commented May 26, 2019

and normalization (which is turned on by default).

Details:
The color map's vmin/vmax was computed based on factors of the whole range
whereas the palette only used the factors from the actually used values.

When using color normalization this lead to:
-- LineCollection normalizing via vmin/vmax computed above, reflecting the
whole range.
-- ListedColormap is transforming the normalized color values provided by the
LineCollection using its own size (N). However that size N was based on the
actually used values and hence too small.

Fix setting edge colors for Chord graphs using an explicit mapping
and normalization (which is turned on by default).

Details:
The color map's vmin/vmax was computed based on factors of the whole range
whereas the palette only used the factors from the actually used values.

When using color normalization this lead to:
-- LineCollection normalizing via vmin/vmax computed above, reflecting the
   whole range.
-- ListedColormap is transforming the normalized color values provided by the
   LineCollection using its own size (N). However that size N was based on the
   actually used values and hence too small.

@philippjfr philippjfr added the bug label May 26, 2019

@philippjfr

This comment has been minimized.

Copy link
Contributor

commented May 26, 2019

Thanks for the fix, it's very much appreciated.

@philippjfr

This comment has been minimized.

Copy link
Contributor

commented May 26, 2019

Presumably for the bokeh backend this was already handled correctly?

@schumann-tim

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2019

Presumably for the bokeh backend this was already handled correctly?
I didn't try it out with bokeh; when I installed holoviews with conda, apparently a too-old version of bokeh was bundled. As I primarily wanted to generate the graphs as svgs I didn't follow up. I can check though.

Would a minimal example be useful for regression testing? (not sure how the current set-up of the project is).

@philippjfr

This comment has been minimized.

Copy link
Contributor

commented May 28, 2019

Would a minimal example be useful for regression testing?

Definitely, you can look at the existing tests in holoviews/tests/plotting/matplotlib/testgraphplot.py TestMplChordPlot for some examples.

@schumann-tim

This comment has been minimized.

Copy link
Contributor Author

commented Jun 1, 2019

To keep the conversation going, here's a repro for the bug.
I'm unfortunately traveling without access to my computer. Will pick this up when i'm back in ~2 weeks.

import os
import pandas as pd
import holoviews as hv
from holoviews import opts, dim

hv.extension('matplotlib')

edges = [{'source': 0, 'target': 2, 'value': 168},
{'source': 0, 'target': 3, 'value': 150},
{'source': 1, 'target': 3, 'value': 50},
{'source': 1, 'target': 2, 'value': 100}]

nodes = [{'index': 0, 'group': 'clonal_id', 'name': 'grey'},
{'index': 1, 'group': 'clonal_id', 'name': 'red'},
{'index': 2, 'group': 'hits', 'name': 'blue'},
{'index': 3, 'group': 'hits', 'name': 'green'}]

Edges reference by the source's node-id -- so include them as well.

color_map = {'grey':'grey', '0': 'grey',
'red': 'red', '1': 'red',
'blue': 'blue', '2': 'blue',
'green': 'green', '3': 'green'}

links_df = pd.DataFrame(edges)
nodes_ds = hv.Dataset(pd.DataFrame(nodes), 'index')
graph = hv.Chord((links_df, nodes_ds), kdims=['source', 'target']).opts(
opts.Chord(
cmap=color_map, edge_cmap=color_map,
edge_color=(dim('source')).astype(str), label_index='name',
node_color=dim('name').str(), normalize=True))

output_path = '/tmp/repro.pdf'
hv.save(graph, output_path)

@philippjfr

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2019

Just going to merge.

@philippjfr philippjfr merged commit 640e96c into pyviz:master Jul 26, 2019

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.005%) to 87.74%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.