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

Recipes #9

Closed
SimonDanisch opened this issue Nov 6, 2017 · 22 comments
Closed

Recipes #9

SimonDanisch opened this issue Nov 6, 2017 · 22 comments

Comments

@SimonDanisch
Copy link
Member

I don't want to derail any further JuliaPlots/Plots.jl#392.

Let's use this issue to collect feedback about various ways to make recipes.

This version of MakiE recipe:

function MakiE.to_mesh(b, m::typeof(lakemesh)) 
    vertices = map(1:size(m.node, 1)) do i
        Point3f0(ntuple(j-> m.node[i, j], Val{2})..., 0)
    end
    triangles = map(1:size(m.elem, 1)) do i
        GLTriangle(Int.(ntuple(j-> m.elem[i, j], Val{3})))
    end
    GLNormalMesh(vertices, triangles)
end

is implemented as

Typ = typeof(lakemesh)
@recipe function f(m::Typ) 
    vertices = map(1:size(m.node, 1)) do i
        Point3f0(ntuple(j-> m.node[i, j], Val{2})..., 0)
    end
    triangles = map(1:size(m.elem, 1)) do i
        GLTriangle(Int.(ntuple(j-> m.elem[i, j], Val{3})))
    end
    GLNormalMesh(vertices, triangles)
end

@mkborregard thinks:

To me, the@recipe macro just signals to me that this is a recipe, and I can trust it to Just Work(TM).

Not sure why the last bit is important. I just think, that the recipe macro is less generic, doesn't compose that well and is unnecessarily opaque. A well known generic construct like overloading a method seems to me much more trustworthy and future proof.

E.g. you can compose them like this:

struct MyType end
to_color(x::MyType) = RGB(rand(), rand(), rand())
lines(x, y, colormap = [MyType() for i = 1:10])

This works, since to_colormap falls back to to_color.(array). Similarly this can be used in other situations. I'm actually already heavily using this kind of composition internally!

Anyways, I don't think that the details are very important right now.
What I plan to do is rethink these concepts freely, and when we do the refactor, I want to reevaluate my solutions and the ones in Plots.jl - and hopefully get a ton of unbiased feedback! :)

Based on that we can decide what we should keep! So it would be best if we just have people open mindedly play around with MakiE, and then just write down what isn't working for them! I will do my best to document those features, so that people trying thinks out have an easy start!

@mkborregaard
Copy link
Contributor

I agree with your suggestion to just play around freely. If this package is ever to replace Plots, of course, it needs to also be compatible with existing recipes everywhere in the package ecosystem.

@SimonDanisch
Copy link
Member Author

Sure, that's pretty important! And it's luckily an orthogonal issue and is hopefully just about implementing the right replacements!

@mkborregaard
Copy link
Contributor

If you do end up suggesting a change of these dynamics I think that will definitely elicit lots of discussion and opinions.

@Evizero
Copy link
Contributor

Evizero commented Nov 6, 2017

Well the macro example above doesn't really show why the macro is useful.

It really is the nested @series definitions and the ability to nicely set default settings for all kinds of kw args that makes it nice (although the later could easily be some Dict) .
obligatory link for convenience: http://docs.juliaplots.org/latest/recipes/.

@SimonDanisch
Copy link
Member Author

Yeah, obviously, which is why I called the above a light version of a recipe ;)
I guess when I was talking about atomic drawing operations I meant defining multiple series!

@mkborregaard
Copy link
Contributor

Also, of course, @recipe only depends on RecipesBase, which is the heart of what makes Plots so useful IMHO. Overloading Makie.to_mesh requires depending on MakiE (or possibly AbstractPlots in some ways of defining that), but that's a wholly different thing.

@piever
Copy link
Contributor

piever commented Nov 7, 2017

On the other hand, this is something that we also do at times: there are several functions in RecipesBase only defined as placeholders so that any package can extend them depending only on RecipesBase. Maybe that approach would work here?
I agree with @Evizero that setting hard values or default values for attributes is a key feature, but that could also be done without macros.

@Evizero
Copy link
Contributor

Evizero commented Nov 7, 2017

but that could also be done without macros.

I don't doubt that. Personally I am neither pro- nor anti-macro. I just made that comment to stress that many more complex visualizations can be realized by composing a number of simple series blocks. So a low degree of verbosity per series seems quite important.

for example this is done with the recipe linked below:

img gif
screen shot 2017-11-07 at 10 19 24 rnd_cartpole

https://github.com/Evizero/RigidBodyRLEnvironments.jl/blob/master/src/cartpole/cartpole_recipe.jl

@piever
Copy link
Contributor

piever commented Nov 7, 2017

That's the cutest plot! I'm not sure whether I'm pro or anti macro either: the current syntax for hard and default values is just too good but OTOH I remember that there was at times a bit too much magic, which made things harder to debug in case something didn't work. My preference would probably be for a macro free verbose way of defining a series with the @series macro as syntactic sugar on top.

@SimonDanisch
Copy link
Member Author

macro free verbose way of defining a series with the @Series macro as syntactic sugar on top.

That's the plan. I will try to push for a non macro based approach, and if it gets too cluttered, I will add some macro sugar on top of it ;)
Maybe I should start by porting @Evizero recipe to a Makie recipe!

@SimonDanisch
Copy link
Member Author

Overloading Makie.to_mesh requires depending on MakiE (or possibly AbstractPlots in some ways of defining that), but that's a wholly different thing.

The function defines a clear interface for everyone, while the macro is pretty opaque and one needs to read through the whole code to figure out what magic it does ;)

Of course, whatever will replace something like RecipeBase (AbstractPlots?) has to be as minimal as possible as well ;)
So not sure what the entire different thing refers to!

@SimonDanisch
Copy link
Member Author

As I understand the code in RecipesBase, this is what happens anyways, but the only function that gets overloaded is apply_recipe! So if I understand correctly, Makies approach is just the generalization of this. I will see how things turn out once I've create my first complex recipe with Makie and how much macro magic I'll need!

@mkborregaard
Copy link
Contributor

Great. Of course if you do choose a completely different system from Plots recipes it may not make sense in the long run to merge MakiE into Plots, but rather keep them as alternative packages. But let's cross that bridge when we reach it.

@SimonDanisch
Copy link
Member Author

it may not make sense in the long run to merge MakiE into Plots, but rather keep them as alternative packages

Not sure why, as long as it's a strict improvement?

@mkborregaard
Copy link
Contributor

mkborregaard commented Nov 7, 2017

Clearly there shouldn't be any problem if it's a strict improvement :-)

It is also clear that there are different tastes - I love the current recipe syntax but it's also very possible what you have in mind will be even better.

@mkborregaard
Copy link
Contributor

I guess it's really hard to say anything sensible before it's more advanced. It's just that you often talk very critically about Plots and I feel inclined to defend it.

@SimonDanisch
Copy link
Member Author

Fair enough! I just try to not waste any time on politeness, and just assume everyone knows that I have a lot of respect for Plots.jl! I know very well where the current shortcomings are coming from and I don't think it was because any developer that worked on Plots.jl was doing a bad job! But I think we do now have a better perspective on things (including myself, who helped creating these short comings and technical debts in a lot of places!)

And I also hope, that when I'm missing the point and criticize something great about Plots.jl for the wrong reasons, that people will point that out quickly. For that to work, I try to do my criticism as publicly and direct as possible, so that people have a chance to step in and correct me.
It's really crucial that my misunderstandings don't make their way into Makie!

So I'm really relying on sensible feedback - and of course, if Makie is not a strict improvement I might as well stop working on it - this is pretty much the one and only premise of the whole project!

@mkborregaard
Copy link
Contributor

👍 to sensible feedback, and to say things directly. I don't think being very public about it helps, as there are probably only 5-6 people in the world who can tell you anything about Plots you didn't know already, and we're all (except the main man) very easy to reach here 😄 . I guess it also feels a little bit unfair because all of us actively engage in supporting the positive hype around MakiE - we want to see this become a great success as much as you do!

@SimonDanisch
Copy link
Member Author

Appreciated! :)

I don't think being very public about it helps

Well I just mean by opening this issue and pinging the 5-6 people ;)

@ChrisRackauckas
Copy link
Contributor

I am very much for making it dispatches instead of a macro-based interface. I think that will make it much easier to debug, and it'll be much easier to introspect.

@juliohm
Copy link
Member

juliohm commented Mar 3, 2018

In a weakly related topic, I'd like to share a limitation of recipe macros that could probably be solved by Makie.jl? The recipe macros currently don't allow things such as splatting as pointed out by @ChrisRackauckas. For example:

using RecipesBase

@userplot FooPlot

@recipe function f(fp::FooPlot; a=1, b=2, kwargs...)
    # SKIP
end

BoundsError: attempt to access 1-element Array{Any,1} at index [2]

Also, I would like to vote +1 to @Evizero's comment on the usefulness of @series macros. They are really convenient. We can compose complex visualizations with ease, and I hope Makie.jl will be able to give users similar composibility.

@asinghvi17 asinghvi17 mentioned this issue Nov 25, 2019
SimonDanisch added a commit that referenced this issue Jun 3, 2021
SimonDanisch added a commit that referenced this issue Jun 3, 2021
make updates via observables work
SimonDanisch added a commit that referenced this issue Jun 3, 2021
@SimonDanisch
Copy link
Member Author

This is outdated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants