dendrograms / astrodendro

Generate a dendrogram from a dataset
https://dendrograms.readthedocs.io/
Other
37 stars 38 forks source link

Short fix to Lasso selection in Scatter, for partially `nan`'d columns #136

Closed tomr-stargazer closed 9 years ago

tomr-stargazer commented 9 years ago

I encountered weird behavior in Scatter when using Lasso select: when I put in catalog data in which either the x or y column (but not both simultaneously) had a few "nan" datapoints, then some incorrect selections would occur when I drew lassos around these data.

I investigated further, and discovered that matplotlib.path.Path.contains_points() actually seems to be at fault here, in that it does the wrong thing: if there are data tuples that are (nan, y) or (x, nan), and "y" or "x" are in the selected lasso range, then those rows get selected. (For example, if you draw a unit circle around the origin, and a row of data has (0, nan), it gets selected by this lasso, although (2, nan) does not.)

The specified pull request is a short fix to this. I've tested it out in production and it appears to do the right thing.

tomr-stargazer commented 9 years ago

@ChrisBeaumont @astrofrog any thoughts on this?

ChrisBeaumont commented 9 years ago

Yes sorry, this looks good. We should report this upstream to MPL as well

ChrisBeaumont commented 9 years ago

Reported at https://github.com/matplotlib/matplotlib/issues/3759

tomr-stargazer commented 9 years ago

ok thanks! (I'm still a little too shy to report things over at matplotlib myself)

mdboom commented 9 years ago

Don't be shy about reporting bugs to matplotlib! We try really hard to be friendly... ;)

Once matplotlib/matplotlib#3759 is merged (and available in a release etc.) this patch is probably no longer necessary.