Skip to content

Commit 1b56739

Browse files
nsicchaclaude
andcommitted
Fix zero-arg IndexableProperty: extract _computeproperty to bypass PropertyCache
IP.call() was returning the cached IndexableProperty instead of computing, because getorcomputeproperty went through PropertyCache get! which had the IP stored. Now call() uses _computeproperty directly, bypassing PropertyCache. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 33d6414 commit 1b56739

1 file changed

Lines changed: 46 additions & 43 deletions

File tree

src/DynamicObjects.jl

Lines changed: 46 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ end
5353
name(::IndexableProperty{N}) where {N} = N
5454
Base.show(io::IO, ip::IndexableProperty{N}) where {N} = print(io, "IndexableProperty :", N, " (", ip.cache, ")")
5555
((;o)::IndexableProperty{name})(indices...; kwargs...) where {name} =
56-
getorcomputeproperty(o, name, indices...; kwargs...)
56+
_computeproperty(o, name, indices...; kwargs...)
5757
Base.getindex(ip::IndexableProperty, indices...; fetch=Base.fetch, kwargs...) =
5858
get!(ip.cache, (indices, (;kwargs...))) do
5959
ip(indices...; kwargs...)
@@ -334,12 +334,54 @@ function _record_accessed_key(o, name::Symbol, indices, kwargs)
334334
_record_key_to_path(path, (indices, (;kwargs...)))
335335
end
336336

337-
getorcomputeproperty(o, name, indices...; __status__=nothing, kwargs...) = if hasfield(typeof(o), name)
337+
_computeproperty(o, name, indices...; __status__=nothing, kwargs...) = begin
338+
vname = Val(name)
339+
# Only pass __status__ to properties that accept it (generated properties
340+
# in meta). Base properties (cache_path, hash, etc.) don't have it.
341+
_status_kw = haskey(meta(typeof(o)), name) ? (; __status__) : (;)
342+
try
343+
if iscached(o, vname, indices...; kwargs...)
344+
cache_path = get_cache_path(o, name, indices...; kwargs...)
345+
mkpath(dirname(cache_path))
346+
cache_status = get_cache_status(cache_path)
347+
rv = if cache_status == :ready
348+
Serialization.deserialize(cache_path)
349+
else
350+
cache_status == :started && @warn "Cache file $cache_path exists but has size 0.\nAssuming a previous run failed."
351+
touch(cache_path)
352+
nothing
353+
end
354+
if cache_status != :ready || resumes(o, vname, indices...; kwargs...)
355+
@debug "Generating $cache_path..."
356+
rv = compute_property(o, vname, indices...; _status_kw..., (name=>rv, )..., kwargs...)
357+
Serialization.serialize(cache_path, rv)
358+
end
359+
# Record accessed key for indexed @cached properties
360+
if !isempty(indices)
361+
_record_accessed_key(o, name, indices, kwargs)
362+
end
363+
rv
364+
else
365+
compute_property(o, vname, indices...; _status_kw..., kwargs...)
366+
end
367+
catch e
368+
e isa PropertyComputationError && rethrow()
369+
kw_tuple = isempty(kwargs) ? () : Tuple(pairs(kwargs))
370+
bt = catch_backtrace()
371+
throw(PropertyComputationError(
372+
string(typeof(o).name.name),
373+
name,
374+
indices,
375+
kw_tuple,
376+
(e, bt), # store exception + backtrace from the actual throw site
377+
))
378+
end
379+
end
380+
getorcomputeproperty(o, name, indices...; kwargs...) = if hasfield(typeof(o), name)
338381
@assert length(indices) == length(kwargs) == 0
339382
getfield(o, name)
340383
else
341384
get!(getfield(o, :cache), name, indices...; kwargs...) do
342-
vname = Val(name)
343385
# When called with no indices on an indexed property (declared with
344386
# call/ref syntax, e.g. `x() = ...` or `x[i] = ...`), return an
345387
# IndexableProperty wrapper instead of calling compute_property.
@@ -349,46 +391,7 @@ else
349391
return IndexableProperty(name, o, subcache(getfield(o, :cache)))
350392
end
351393
end
352-
# Only pass __status__ to properties that accept it (generated properties
353-
# in meta). Base properties (cache_path, hash, etc.) don't have it.
354-
_status_kw = haskey(meta(typeof(o)), name) ? (; __status__) : (;)
355-
try
356-
if iscached(o, vname, indices...; kwargs...)
357-
cache_path = get_cache_path(o, name, indices...; kwargs...)
358-
mkpath(dirname(cache_path))
359-
cache_status = get_cache_status(cache_path)
360-
rv = if cache_status == :ready
361-
Serialization.deserialize(cache_path)
362-
else
363-
cache_status == :started && @warn "Cache file $cache_path exists but has size 0.\nAssuming a previous run failed."
364-
touch(cache_path)
365-
nothing
366-
end
367-
if cache_status != :ready || resumes(o, vname, indices...; kwargs...)
368-
@debug "Generating $cache_path..."
369-
rv = compute_property(o, vname, indices...; _status_kw..., (name=>rv, )..., kwargs...)
370-
Serialization.serialize(cache_path, rv)
371-
end
372-
# Record accessed key for indexed @cached properties
373-
if !isempty(indices)
374-
_record_accessed_key(o, name, indices, kwargs)
375-
end
376-
rv
377-
else
378-
compute_property(o, vname, indices...; _status_kw..., kwargs...)
379-
end
380-
catch e
381-
e isa PropertyComputationError && rethrow()
382-
kw_tuple = isempty(kwargs) ? () : Tuple(pairs(kwargs))
383-
bt = catch_backtrace()
384-
throw(PropertyComputationError(
385-
string(typeof(o).name.name),
386-
name,
387-
indices,
388-
kw_tuple,
389-
(e, bt), # store exception + backtrace from the actual throw site
390-
))
391-
end
394+
_computeproperty(o, name, indices...; kwargs...)
392395
end
393396
end
394397
maybehash(x::Number) = x

0 commit comments

Comments
 (0)