Makie.jl
Makie.jl copied to clipboard
Improve and use `shared_attributes` for attribute passthrough
Description
We have shared_attributes(parent, TargetType)
as a function that filters attributes from parent
applicable to TargetType
but we are not using it. This pr aims to extend the function and use it consistently in recipes. That should help fix a lot of the issues about attributes not being passed along/having no effect in recipes.
Some thoughts:
- Allowing
shared_attributes
to skip/drop a set of given attribute names means that some valid attributes will not get set. That sounds like something we should not encourage. Edit: It is quite useful in some cases, e.g. if you want to pass strokecolor to a poly but not a scatter or fxaa to a mesh but not lines. - Renaming attributes is a common and valid task (e.g.
strokecolor => color
), soshared_attributes
should handle that - Replacing attributes is also common, but typically requires a multiline
map
(etc). Because of that I think it's preferable to haveattr[:name] = map(...)
instead of something likeobs = map(...); shared_attributes(..., :name => obs)
. - There are some attributes that only make sense at the top level, e.g. the
inspector
ones. These should be automatically cleared. - Warnings for missing valid attributes might be useful to find further issues with missing attribute passthrough.
First Version
The usage of the first version looked something like this:
# filters attributes applicable to TargetType from parent plot
# remove some top level/local attributes
# renames based on rename list
attr = shared_attributes(parent, TargetType, [:name_in_parent => :name_in_target, ...])
# removing attributes
pop!(attr, :name)
# replacing/adding
attr[:name] = new_attr
This has a couple of problems:
- External pop! and replacing/adding makes "warn_on_unset_attribute" pretty useless since a good chunk of attribute mutation happens afterwards
- Replacements invite mistakes as
setindex!(::Attributes, val, name)
updates Observables if val is not an Observable. E.g.lines_attr[:fxaa] = false
would change the parent attributes fxaa value, which could also be used in another plot where it should remain true. - The rename array is unintuitive. As a replacement it would be written in reverse order as
attr[:name_in_target] = parent[:name_in_parent]
Second Version (c04f450 and after)
attr = shared_attributes(
parent, TargetType,
:name_to_delete, ...;
name_to_replace_or_add = value, ...
)
The second version solves the issues above:
- Everything can/should happen in the
shared_attributes
call so we can track defaulted attributes. - Replacements (given as kwargs) are handled by
shared_attributes
to not change values of the parent. - Renaming doesn't exist explicitly anymore. They are handled via replacements/kwargs, i.e.
new_name = parent[:old_name]
.
Related Issues/prs
- Fixes #4458
- Closes #2206 (shared_attributes should be a preferable alternative)
- Fixes #2733
- Closes #3263
- Closes #4435
Type of change
- Internal cleanup with potential for breaking changes in attribute passthrough
Checklist
- [x] Added an entry in CHANGELOG.md (for new features and breaking changes)
- [ ] Added or changed relevant sections in the documentation
- [ ] Added unit tests for new algorithms, conversion methods, etc.
- [ ] Added reference image tests for new plotting functions, recipes, visual options, etc.