PlotUtils.jl icon indicating copy to clipboard operation
PlotUtils.jl copied to clipboard

Problems with conversion of `Unsigned` to `RGB`

Open mateuszbaran opened this issue 4 years ago • 5 comments

I've accidentally been using Base.convert method from this package that converts Unsigned to RGB. This is type-unstable (which is easy to fix) but it also looks like type piracy, so this method likely shouldn't even be in this package. Here is that method: https://github.com/JuliaPlots/PlotUtils.jl/blob/44e45ec395afb6eaf84592f72247cdba30409546/src/color_utils.jl#L96-L99 . What do you think should be done about it?

mateuszbaran avatar Aug 23 '21 13:08 mateuszbaran

@BeastyBlacksmith, do you know why this is exposed in Base ?

t-bltg avatar Aug 25 '21 11:08 t-bltg

This predates my time quite a bit. But I agree, that this is type piracy and we should get rid of it. One just has to be careful with this, since it is unclear where this is used. It might not only affect Plots, if we remove it.

BeastyBlacksmith avatar Aug 25 '21 12:08 BeastyBlacksmith

This predates my time quite a bit. But I agree, that this is type piracy and we should get rid of it. One just has to be careful with this, since it is unclear where this is used. It might not only affect Plots, if we remove it.

Thanks.

@mateuszbaran you can make a PR changing this to an internal method, but you'll have to run Plots.jl test suite, checking that this does not break anything. Someone should also investigate where this method is used in the JuliaPlots ecosystem, to find potential breaks.

t-bltg avatar Aug 25 '21 21:08 t-bltg

Here is a list of all dependents: https://juliahub.com/ui/Packages/PlotUtils/YveHG/1.0.11?t=2

BeastyBlacksmith avatar Aug 26 '21 07:08 BeastyBlacksmith

That's quite a lot of dependent packages. I could check if removing it breaks Plots.jl but going through all of these packages is a bit too much :sweat_smile: .

mateuszbaran avatar Aug 26 '21 09:08 mateuszbaran