epfl-lts2 / pygsp

Graph Signal Processing in Python
https://pygsp.rtfd.io
BSD 3-Clause "New" or "Revised" License
488 stars 93 forks source link

Add specifying edge color with RGBA array #41

Closed jbcdnr closed 5 years ago

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.2%) to 86.426% when pulling f1ef6d015a1e355f45a352ae5342da57aef1c8dd on jbcdnr:master into 4f9f897eb68bb88b8f550c06c0fd624a512d6397 on epfl-lts2:master.

jbcdnr commented 5 years ago

@mdeff I updated the doc.

Also I fixed the default behaviour for 1D signal: should be considered as a signal and not a grey scale by Matplotlib. Let me know if you see a cleaner way to check this condition than try except (bede579e357df9dafa7e760547a766d152f24546).

mdeff commented 5 years ago

What's the issue with 1D signals? I've tested with 1D signals, and all signals that are not of length 3 or 4 (where there's an unsolvable ambiguity) aren't considered colors. Single numbers are not considered color either.

>>> mpl.colors.is_color_like(0.8)
False
>>> mpl.colors.is_color_like((0.8,)*1)
False
>>> mpl.colors.is_color_like((0.8,)*2)
False
>>> mpl.colors.is_color_like((0.8,)*3)
True
>>> mpl.colors.is_color_like((0.8,)*4)
True
>>> mpl.colors.is_color_like((0.8,)*5)
False
>>> mpl.colors.is_color_like((0.8,)*10)
False

Also, no test failed without that commit.

jbcdnr commented 5 years ago
>>> mpl.colors.is_color_like((0.8,)*4)
True

1D signal of values in [0, 1] of length 3 or 4 would be considered as grey scale and would not be normalised as expected (> 0.25). The bug is also present in master and would be fixed with this. However the control flow is not very elegant.

mdeff commented 5 years ago

I see. That's indeed the unsolvable ambiguity. It happens with pg.graphs.Sensor(3, k=1).plot([1, 0, 0]). Matplotlib warns about the situation if we run pg.graphs.Sensor().plot([1, 0, 0]).

'c' argument looks like a single numeric RGB or RGBA sequence, which should be
avoided as value-mapping will have precedence in case its length matches with 'x' & 'y'. 
Please use a 2-D array with a single row if you really want to specify the same RGB
or RGBA value for all points.

I think that the corner case it covers (graphs with 3 or 4 nodes or edges) is not worth the complexity. Moreover, the case can be resolved on the user side by following matplotlib's advice.

I simplified in 3f4d4c4a2ef0ccc05063a4e06be8632622d4e295. Thanks for your contribution! :)