Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Clean up plot attributes #1388

Closed
4 of 11 tasks
SimonDanisch opened this issue Oct 15, 2021 · 5 comments
Closed
4 of 11 tasks

Clean up plot attributes #1388

SimonDanisch opened this issue Oct 15, 2021 · 5 comments
Labels
Attributes Plot, Block and Scene Attributes Collection contains multiple issues documentation enhancement Feature requests and enhancements Makie Backend independent issues (Makie core) maybe outdated/fixed TODO: review plot Related to plot object

Comments

@SimonDanisch
Copy link
Member

SimonDanisch commented Oct 15, 2021

  • go through all basic plot types and make sure they have a consistent, well defined set of attributes
  • move all color related attributes to color (e.g. colormap)
  • consistently allow for the same attributes in colormap (e.g. high_clip, etc)
  • add alpha attribute support alpha keyword arg #84
  • move all shading attributes to e.g. material
  • move all light attributes to scene
  • make sure all attributes are documented and tested
  • make sure it's clear what types are allowed for each attribute
  • rename backend attributes like fxaa, to make the cross backend, or move them to something like backend-attributes
  • good error messages for wrong plot attributes
  • better API for backends. Right now, it's pretty fuzzy to see what the basic plot types support, leading to bugs + making it more difficult to write new backends/maintain them

Related issues:
#1230
#879
#744
#84
#545

@SimonDanisch SimonDanisch added the Makie Backend independent issues (Makie core) label Oct 15, 2021
@ffreyer
Copy link
Collaborator

ffreyer commented Oct 15, 2021

We should also clean up the Attribute handling in recipes. Currently some recipes inherit attributes, some don't, some are probably missing attributes and passing them along is often very explicit.

For example arc inherits with default_theme(scene, Lines) and passes Attributes(p), whereas spy doesn't inherit any attributes (which might be reasonable), is missing some general attributes like visible and passes explicitly with name = p.name.

In that context it might also be good to cleanup/change default_theme. Currently it has some defaults for attributes that belong to every plot like visible, some that apply to a category like diffuse (only relevant for 3D plots) and some that target one or two specific plot primitive like linewidth. Having default_theme(scene) return just generally applicable attributes and default_theme(scene, Plot2D/Plot3D) for 2D and 3D specific attributes would make writing recipes easier I'd say. A constructor like Attributes(parent...; kwargs...) = merge(parents..., kwargs) would also be useful I think.

@ffreyer
Copy link
Collaborator

ffreyer commented Oct 19, 2021

make sure all attributes are documented and tested

Maybe we can use the @documented_attributes from MakieLayout (or something similar) for plots as well? Maybe also with inheritance, i.e.

default_attributes = @documented_attributes begin
    "Turns a plot in/visible. "
    visible = true
    ...
end
default_3D_attributes = @documented_attributes begin
    default_attributes
    "Sets the brightness of ambient light. The components of this vector act as multipliers to the red, green and blue color values of the plot."
    ambient = Vec3f0(0.55)
    ...
end
@recipe(Mesh, mesh) do scene
    @documented_attributes begin
        default_3d_attributes(scene)
        "Sets the flat color, vertex color or texture of the mesh depending on the passed type (color type, Vector or Matrix of colors respectively)."
        color = :black
        ...
    end
end

to generate a docstring like this:

{other dosctring stuff}

Attributes:
- `visible = true`: Turns a plot in/visible.
...
- `ambient = Vec3f0(0.55)`: Sets the brightness of ambient light. The components of this vector act as multipliers to the red, green and blue color values of the plot.
...
- `color = :black`: Sets the flat color, vertex color or texture of the mesh depending on the passed type (color type, Vector or Matrix of colors respectively).
...

@jkrumbiegel
Copy link
Member

Yeah something like that would be good. I've been cleaning up in that MakieLayout code a bit in my layoutables PR, and we can definitely share some of that code with the recipe code that Simon is refactoring.

What would be a good way to access help for attributes? Currently I dump everything into one large docstring but that gets pretty difficult to read after a while.

@ffreyer
Copy link
Collaborator

ffreyer commented Oct 19, 2021

I think in some cases nesting attributes could be helpful. More specifically for Axis a lot of the x/y attributes could also be combined. E.g.

axis.grid = Attributes(
    color = (RGBA(0,0,0,0.12), RGBA(0,0,0,0.12)),
    style = (nothing, nothing),
    visible = (true, true),
    width = (1.0, 1.0)
)

and document as

- `grid = Attributes(...)`: Holds onto various grid attributes.
    - `color = (RGBA(0,0,0,0.12), RGBA(0,0,0,0.12))`: The color (x, y) grid lines.
    - `style = (nothing, nothing)`: The linestyle of (x, y) grid lines.
    - `axis.grid.visible = (true, true)`: Controls if the (x, y) grid lines are visible.
    - `width = (1.0, 1.0)`: The width of the (x, y) grid lines.

I think for plots it's probably fine to just dump all that information as is. Scatter has 30 attributes, compared to ~120 from Axis, and some of them shouldn't be there (lighting stuff) or are backend attributes (uv_offset_width? distancefield?) which don't need to appear in a docstring.

@ffreyer ffreyer added enhancement Feature requests and enhancements documentation plot Related to plot object Collection contains multiple issues Attributes Plot, Block and Scene Attributes maybe outdated/fixed TODO: review labels Aug 22, 2024
@ffreyer
Copy link
Collaborator

ffreyer commented Aug 22, 2024

checked off some points here (none were checked before)

@MakieOrg MakieOrg locked and limited conversation to collaborators Aug 22, 2024
@ffreyer ffreyer converted this issue into discussion #4206 Aug 22, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Attributes Plot, Block and Scene Attributes Collection contains multiple issues documentation enhancement Feature requests and enhancements Makie Backend independent issues (Makie core) maybe outdated/fixed TODO: review plot Related to plot object
Projects
None yet
Development

No branches or pull requests

3 participants