Plots recipe

[tbreloff]

I have a possible solution to JuliaPlots/Plots.jl#460 on the Plots dev branch, and this tiny PR is all you'd need (in theory) for flexible plotting of arrays of Quantity. Here's an example:

using Plots, Unitful
pyplot(leg=false)
°C = u"°C"

plot(
    plot(100rand(10) .* °C),
    surface(100rand(10,10).*°C, right_margin=30px)
)

I know you may be hesitant to include the RecipesBase dependency, but it's about as lightweight as dependencies come.

cc: @ChrisRackauckas

[ajkeller34]

Thanks for focusing your efforts on this! I'd be delighted to have this functionality. I did some reading on how conditional dependencies should be handled, and came across the following recent thread: https://groups.google.com/d/msg/julia-users/RDTZnVbiBig/9wufXnFgBAAJ

Based on Tony Kelman's comments and, well, my hesitation to make the jump from zero to one dependency even if it is lightweight, would you mind resubmitting this PR to https://github.com/ajkeller34/UnitfulPlots.jl? That package requires both Unitful and RecipesBase and is intended for this purpose. Perhaps you can wrap what I have there in some version checking for the current version of Plots, and this PR in version checking for the next version of Plots (via Pkg.installed("Plots"))?

From the sound of Tony Kelman's comment, it seems like eventually Julia will have a better mechanism for conditional dependencies, at which point I'd be happy to ditch the two package solution, but I think this is the safest/cleanest for now. I can register/tag UnitfulPlots and make a prominent note of UnitfulPlots in the README and docs for Unitful if these are concerns. I put in a PR for registering and tagging Unitful earlier today.

[ChrisRackauckas]

What's being talked about for conditional dependencies there isn't relevant to this. RecipesBase would be a standard dependency. All that would be added is one that one repo, not Plots.jl.

You shouldn't have to check if Plots is installed for this.

[ajkeller34]

I understand that I would not have to check for Plots. I also wouldn't have to check for RecipesBase if it were a standard dependency. But it should not be a standard dependency, because literally 99.9% of Unitful doesn't depend on it. So yes, the discussion is relevant. I don't think it's too much to ask to have users who want the functionality do a one liner: Pkg.add("UnitfulPlots"). I would point out that UnitfulPlots is also lightweight.

[ChrisRackauckas]

Oh I see. You don't want RecipesBase as a dependency. I think it's such a small dependency it shouldn't matter, but yes, if you want it only conditionally applied, then it would have to be added to UnitfulPlots because the method of conditional dependencies described doesn't work well for macros.

[ajkeller34]

Yes, it is a small dependency, but this is not really Unitful's responsibility. After all, Unitful has nothing to do with plotting. Take this to extremes: any time some package has functionality that could be extended with unit support, should that mean that Unitful needs a PR? I think that's a backwards design: in my ideal world Plots would check to see if unit support is enabled by a conditional dependency, either Unitful or SIUnits or both, and declare appropriate code based on that to support units in plotting. But since conditional dependencies are annoying to do well at the moment, I think UnitfulPlots or something like it is a fair alternative.

[tbreloff]

Sure... Just copy paste wherever you want this. I think one line of code is
pretty silly to justify a whole package though...

On Wednesday, September 21, 2016, Andrew Keller notifications@github.com
wrote:

Yes, it is a small dependency, but this is not really Unitful's
responsibility. After all, Unitful has nothing to do with plotting. Take
this to extremes: any time some package has functionality that could be
extended with unit support, should that mean that Unitful needs a PR? I
think that's a backwards design: in my ideal world Plots would check to see
if unit support is enabled by a conditional dependency, either Unitful or
SIUnits or both, and declare appropriate code based on that to support
units in plotting. But since conditional dependencies are annoying to do
well at the moment, I think UnitfulPlots or something like it is a fair
alternative.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#21 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA492sXdTN2z2CB9EyKCIEdnmYtbJz06ks5qsMvNgaJpZM4KCHEu
.

[tbreloff]

To expand on my brief comment in the middle of the night... I completely understand that you don't want to add the RecipesBase dependency, because it's oh-so-nice to have zero deps. But in this case, its also extremely silly to add the annoyance of an extra package just for holding that dependency and one line of code. (Packages have cost... you're going to find it annoying to constantly tell people to install/load a package they don't know about)

So I changed the PR so that there's no dependency, and also no need for UnitfulPlots.jl. I tested and it seems to work fine.

[ajkeller34]

I just tested it and there is a problem: since Unitful is precompiled, if RecipesBase is installed after Unitful, then you won't get the functionality. This approach was explicitly mentioned in the comment thread I found, and deemed problematic for this reason. See also another comment at the end of JuliaLang/julia#16970:

"The only reliable way to do optional dependencies and precompilation right now is to separate the code that depends on both packages being present into a third package that can unconditionally depend on both. Until extern gets implemented, anyway."

Look, I hear what you're saying about it being silly to have an extra package for one line of code, and it is silly, but deep down you have to recognize that this is something that should be a conditional dependency, not an unconditional one. It's not my fault this situation isn't well supported yet in Julia.

Besides, it is never just one line of code, is it... what about all the other packages that need some tweaks to play nicely with Unitful? This is a broader problem. I'll rename UnitfulPlots to UnitfulSupport. In this package, I will put all code that is required to make other packages play nicely with Unitful. I can think of another example right off the top of my head: what about ForwardDiff? At present you can't take the derivative of a Quantity-valued function. Should I require ForwardDiff in Unitful to fix that? Probably not. And eventually, once conditional dependencies are robustly implemented, I will take great joy in deprecating UnitfulSupport.

[ChrisRackauckas]

Yeah, and even though in many cases this form of conditional dependency works, Tony Kelman will block your PR for awhile if you have it because of that precompilation issue (I don't know why he hasn't gotten mad at Plots for this yet...). It does have an easy user fix (delete precompilation cache, done), but the real fix is for true conditional deps to be in Base. @tbreloff you may really want to push for this harder so it gets in far before v0.6, because I think you might have a lot of code to change with however it gets implemented.

JuliaLang/julia#6195
JuliaLang/julia#15705

[ChrisRackauckas]

For the extended discussion (post-julia-users thread):

JuliaLang/METADATA.jl#5761
JuliaLang/METADATA.jl#5765

Given that you see how he will go against any time I mentioned this form of conditional dependency (and that there are cases where this method does fail), it's wise to not start this uphill battle and instead just really push for a Base solution.

[tkelman]

and that there are cases where this method does fail

like your own JuliaCI/PackageEvaluator.jl#138 - I don't know which package is responsible for this one, but I'm going to continue recommending against more packages doing similar unreliable things. You can disable precompilation on all the relevant packages, as an alternative.

[ChrisRackauckas]

@ajkeller34 a similar issue is coming up in this PR if you're interested:

GiovineItalia/Gadfly.jl#902

Yes, that PackageEvaluator error is frustrating. I don't know if it's possible to just tell everyone to not precompile if they have to do some form of conditional dependency, because then that breaks any package which depends on them and precompiles.

[tkelman]

Yes. Conditional dependencies break precompilation. That's exactly what I've been saying.