Cleanup nested attributes (docs, validation, shared_attributes, updating, infrastructure)#5620
Open
ffreyer wants to merge 54 commits into
Open
Cleanup nested attributes (docs, validation, shared_attributes, updating, infrastructure)#5620ffreyer wants to merge 54 commits into
shared_attributes, updating, infrastructure)#5620ffreyer wants to merge 54 commits into
Conversation
Collaborator
Benchmark ResultsSHA: 0f0ae76ff1c10d5d2a359dc0e104508cddd9a599 Warning These results are subject to substantial noise because GitHub's CI runs on shared machines that are not ideally suited for benchmarking. |
probably incomplete...
…gendEntry generation
5 tasks
ffreyer
commented
May 15, 2026
| Base.setproperty!(::Input, ::Symbol, ::Computed) = error("Setting the value of an ::Input to a Computed is not allowed") | ||
|
|
||
| function Input(graph, name, value, f, output, force_update = false) | ||
| validate_node_value(value) |
Collaborator
Author
There was a problem hiding this comment.
For context, validate_node_value(value) causes type inference which costs about 2µs of ~45µs total in @benchmark scatter!(scene, 1:5) setup=(scene = Scene()).
Iirc I added this initially to make sure add_input!() is going down the correct paths. Under normal usage it should never trigger. The calls in Computed are never reached.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
I noticed that I basically just implemented the backend for nested attributes in #5482. They worked with
Attributesin the old@recipe ... do scene ... endsyntax, but not in@recipe ... begin ... endor@Block. As a result they didn't integrate withDocumentedAttributes, i.e. didn't have docstrings, validation and mixins. There were also some integration with other features missing, likeshared_attributes(), nested attributes/compute graph passthrough or nested attribute updating with merging viaplot.nested = Attributes(...).While working on this I also got sucked into a bunch of tangential things:
First, while we have been using new recipes for around 2 years, old recipes still existed without any deprecation warning and as the thing that
?@recipedocuments. So I updated the docstring and added a deprecation warning.Second, old, new and block recipes each had their own attribute infrastructure. This means a bunch of repetition between new recipes and block recipes and bunch of old vs new paths with old vs new recipes. I reworked all of this to feed into the same
@DocumentedAttributesinfrastructure so it can then be handled by the same processing. (Well, more or less. Plots initialize their attributes in two parts, first graph init with passthrough and kwargs, then theme resolution, while blocks do it all in one step. To not slow down blocks, these bits are still a little different.)This somewhat naturally lead to reworking the attribute init code of plots, which was a bit messy before imo and got extra messy after adding nested attributes because I added a bunch of extra branching and recursion.
Third was me getting frustrated with performance after the pr benchmarks starting looking worse. I had been thinking for a while that I should maybe rework
DocumentedAttributesto do some of the nesting workComputeGraphat@DocumentedAttributesresolution time and then save things in a flattened format. I tried and got ~20% faster create benchmarks than the previous commit, so I went ahead with. That meant another reworkAt some point I remembered that SpecApi exists. I updated
BlockSpecto accept arguments (for block/complex recipes) and updated (and unified) the block and plot infrastructure to work with nested attributes.What changed
Recipes
Old Recipes now warn about being outdated during
@recipeevaluation and convert their attribute callback to a new recipe style block, before feeding it to the@DocumentedAttributesinfrastructure. I tried to make this fully compatible, but I feel like that's impossible when you're converting a user define callback...?@recipegot update to document the "new" style of recipes@recipe ... begin ... end.plot.outer = Attributes(...)should now allow for nested attributes to be updated in bulk.shared_attributes()is now compatible with nested attributes + new infrastructureBlock Recipes
No longer changes
attribute_namesto force validation to accept kwargs. That now happens inblock_kwargs.More or less all of the old attribute processing and docs handling has been removed and replaced by
@DocumentedAttributes. This should be internal, though I wouldn't be surprised if the removed functions affect some other packages...Legend
Legends used to overwrite
Legend()for argument processing which requiredblock_defaultsto fill out attributes early so that entry groups could be initialized. This blocked the removal ofblock_defaultsso I reworked it to process arguments ininitialize_block!().I ran into a bunch of issues with the newer
MeshElementetc. and more generally the legend element attribute infrastructure here which I also (tried to) fix. I think it would be worth it to rework this further with nested attributes, to replace attributes likeLegend.markerstrokewidthwithLegend.scatter.strokewidthand remove all the renaming it required. But that's probably something for another pr...@DocumentedAttributes(user facing)"New" recipes and block recipes now feed into the
@DocumentedAttributesdirectly. As such they have the same user facing interface. (Old recipes still have their own interface/syntax, which gets translated.)Nested attributes are supported with
@attributesblocks, which can also have a docstring:Mixins (the ... parts) can now be used in nested scopes. (For compat with old recipes they can also run be variables now, though
escrules may make them impossible to use outside of that context.)@inheritnow supports callbacks (old recipe, block compat), multiple nested inherits (block compat) and inheriting from nested theme entries (block compat). There is also compat forinherit()(blocks) andmap()(blocks, old recipes compat earlier) andtheme(scene, key)here.Another minor note here is that adding no fallback now results in
NoFallback()rather thannothingso thatnothingis now a valid fallback value.Types annotations can now be added to attribute names. These are only used by blocks, but can be added in plot recipes too.
DocumentedAttributes(internal)I burned down the house and build a new one.
DocumentedAttributesare no longer a wrapper around aDict. Instead they are a collection of flattened arrays withComputePipeline.NestedSearchTreeencoding the nesting.Generation of
DocumentedAttributesnow includes some state to encode the nesting (basically the recursion stack you'd have with recursive functions) and uses_begin_nesting_layer!(),_end_nesting_layer!(),_add_attribute!()and_merge_attributes!()to build the structure. Parsing still more or less runs through the same functions, but relies much more onMacroTools.Plot Infrastructure
Plots now step through:
Plot(): extract attributes in first argumentbuild_plot(): argument initadd_attributes!(): branching forcycleinit_graph!(): apply/add documented_attributes. kwargs and passthrough attributesThe latter calls into new
DocumentedAttributesinfrastructure:prepare_graph_for_attributes!(): copies nesting information into compute graph and adds headless compute nodes for all missing attributesadd_from_kwargs!(): adds Inputs or connections to parent compute nodes for each headless node based on kwargsconnect_parents!(): connects headless nodes to appropriate nodes in parent graphadd_prepared_input!()add_remaining_inputs!()adds inputs for the remaining headless nodes. These may containInherit(...)as this makes no effort to resolve theming. (There is no scene here, so why bother?)Steps 2-5 all rely on a
build_callbackfunction defined inadd_attributes!()to create edge callback functions. With a bit of help from@nospecializethe attribute initialization gets away with (almost?) no specialization/type inference at all. Only kwargs check for Observable vs Computed vs value.Theming is resolved in
add_theme!()(like before) by another function of the same name. Instead of stepping through kwargs, an overwrite theme, documented attributes and then the default theme for each attribute, it builds a vector of resolved defaults withresolve_defaultsfirst and then applies those. So it does one pass through kwargs, filling relevant bits of the vector, then overwrite, etc. This should help a bit with data locality and reduce type inference/checks for nested attributes (if entry isa Attributes; recurse). This also dodges type inference/specialization of defaults. (They are ::Any typed inInputso knowing their type is useless.)Block Infrastructure
To initialize block attributes
_blockcalls:resolve_defaultsto get initial values for attributesadd_attributes!()with those defaults, as a user-overwrite path (I added this in the Axis compute graph rework)prepare_graph_for_attributes!()which initializes nodes in groups of the same type here. This should reduce the amount of type inference (once per unique type instead of once per attribute)add_remaining_block_inputs!()which runs through steps 2, 4, 5 andadd_theme!()in one go.SpecApi Infrastructure
Both
PlotSpecandBlockSpecnow rely onbacth_update*!()functions to collect a list of attributes to update. This should do the same thing as before, but in a nested-attribute compatible way.Note that this code is not optimized. It may be beneficial to flatten out
kwargsin both specs to avoid recursion inbatch_update*, and it may be beneficial to save the resolved defaults to avoidlookup_defaultcalls when kwargs get unset.Documentation
The
$ATTRIBUTESthing andhelp_attributesthat got left behind for old recipes in #5389 is now gone, as old recipes integrate withDocumentedAttirbutes.Pretty much all of the code around attribute docs got reworked to work with the new
DocumentedAttributesformat. I.e. generating code for the attribute section in?Plotand the online docs as well as thefield_docsfor?Plot.attributeandhelp(Plot, :attribute).In
?Plot, groups are now defined based on nested paths, e.g. if:outeris part of a group, every inner attribute is also part of the group. Those nested attributes are shown asouter.(a, inner.(a, b), c). For ungrouped attributes I got a bit lazy, so they just show as their merged keysouter.a, outer.inner.a, outer.inner.b, outer.c.In online docs generate mostly the same output. Nested Attributes create the same kind of section as unnested ones, but with their merged key as a title, i.e. "#### `outer.inner.a`".
I believe attribute examples should parse and integrate fine if they use the merged key as their header. I.e. "### `outer.inner.a`".
help()now accepts multiple keys to trace a nesting path. If that path is complete (points to a leaf attribute) it displays the same thing as it used to with the merged key as the attribute name. If the path is incomplete (i.e. there is more nesting), it displays (for example):Removed Functions/structs
attribute_names,_attribute_docs(both)plot_attributes(plots)default_attribute_values,attribute_types,attribute_default_expressions,block_defautlts(blocks)_attribute_list,repl_docstring,REPL.fielddoc(blocks, replaced by shared docs infrastructure)make_attr_dict_expr,extract_attributes!,inherit(blocks, replace by@DocumentedAttributesinfrastructure)help_attributes(old recipes, superseeded byhelpwithDocumentedAttributesintegration)$ATTRIBUTESDocStringExtension thing (old recipe)AttributeMetadataand associated functions (fairly useless for new system)LegendOverridewrapper (doesn't seem necessary and internal)apply_theme!()(plots, unused)Cleanup
attribute_docs()functions (unused, I think these were vibe coded in improve documentation #5389)TODO
plot.outer = Attributes(a = 1)works and merges correctlyDocumentedAttributes@inheritwith callbacks