allow combining histogram with visual(Stairs)

[simeonschaub]

and other plot types such as Scatter. Possibly an alternative to #572.

The only drawback here is that the visual(...) needs to come before
histogram(), otherwise an error is thrown because width is not
mapped for other plot types. What do you think of this approach?

[jkrumbiegel]

I've thought about similar things as well, I'm not sure if visual is a good idea here. I think it's a bit convoluted to understand that a transformation might react to a plot type attached to a layer before it is applied, because most transformations come with their own plot types (like histogram does as well). Post-multiplying a visual is ok because you're just changing the output of the transformation as expected. But as your code reflects, you can't of course simply reuse the input data for a histogram for a stairs plot and expect that to work.

I can think of two options. One would be to instead multiply with a visual that does actually look like a stairs version of a barplot. This would have to be added to Makie.

The other option is to add a keyword to the transformation itself in order to choose the desired plot primitive. Then I would have no problem with modifying the data internally, that's entirely expected. I think in general this will have to be done for some transformations because they can create multiple layers with different visuals. If you have that, post-multiplying with visual(some_plottype) will never work. I just haven't come up with a way yet that I would like such an interface to look, because you'd generally want to be able to edit all visual properties of the layers that a transformation creates. I had been thinking of actually just passing a visual as a keyword argument. Like histogram(visual = visual(Stairs, alpha = 0.3)) where there could be multiple different ones for transformations with multiple layers.

[simeonschaub]

You are right, the more I thought about it the less convinced I was that my initial proposal was the right way to go. I now tried implementing your second suggestion with the visual keyword argument, what do you think?

[simeonschaub]

Bump :)

[jkrumbiegel]

This would also need an addition to the docstring of histogram and probably a docs example as well.

My only hesitation is that I haven't yet thought about what to do in multi-layer transforms and if those should share an interface with this situation here, but maybe that's premature API optimization, in general passing a Visual seems sensible to control exactly what it is, plottype + attributes.

[simeonschaub]

Thanks for the comments and suggestions! I exported Visual for now so we can use it here to simplify the logic, I hope that's fine.

My only hesitation is that I haven't yet thought about what to do in multi-layer transforms and if those should share an interface with this situation here, but maybe that's premature API optimization, in general passing a Visual seems sensible to control exactly what it is, plottype + attributes

Hmm, yeah, I must admit I haven't thought about the multi-layer case until now, what kind of applications were you thinking of? As you said though, I would probably go with this for now and we can always revisit when it comes up again.

[jkrumbiegel]

Ok I've thought about this again and I think Visual is not quite right here after all. The reason being that the visual attributes are not necessary to control the computation of the histogram data for the given plottype. For that you only need the type. And there's already the documented way of merging in attributes by appending a * visual(...) so we don't need to duplicate that for now by exporting Visual which is sort of confusing anyway. Maybe later with multilayer transformations. But for this case, a plottype argument is probably the easiest overall. This could also be an optional positional arg. I kind of like histogram(Stairs). What do you think?

[simeonschaub]

Yes, agree that's better! Implemented the changes

[simeonschaub]

Bump