-
Notifications
You must be signed in to change notification settings - Fork 21
Closed
Description
Both getproperties and setproperties are documented to relate to struct properties and not its fields.
However, the implementation is very inconsistent in fields vs properties.
Calling fieldnames and then getproperties:
ConstructionBase.jl/src/ConstructionBase.jl
Lines 47 to 49 in 2044dd5
| fnames = fieldnames(obj) | |
| fvals = map(fnames) do fname | |
| Expr(:call, :getproperty, :obj, QuoteNode(fname)) |
Reporting
fieldnames in the error message:| Failed to assign properties $(fieldnames(P)) to object with fields $(fieldnames(O)). |
These inconsistencies lead to confusing errors, e.g.:
julia> using ConstructionBase, StructArrays
julia> s = StructArray(a=1:5)
5-element StructArray(::UnitRange{Int64}) with eltype NamedTuple{(:a,), Tuple{Int64}}:
(a = 1,)
(a = 2,)
(a = 3,)
(a = 4,)
(a = 5,)
julia> fieldnames(typeof(s))
(:components,)
julia> propertynames(s)
(:a,)
julia> getproperties(s)
ERROR: type NamedTuple has no field components
# ConstructionBase tries to call getproperty(s, :components) which clearly doesn't existMaybe, getproperties
ConstructionBase.jl/src/ConstructionBase.jl
Lines 46 to 53 in 2044dd5
| @generated function getproperties(obj) | |
| fnames = fieldnames(obj) | |
| fvals = map(fnames) do fname | |
| Expr(:call, :getproperty, :obj, QuoteNode(fname)) | |
| end | |
| fvals = Expr(:tuple, fvals...) | |
| :(NamedTuple{$fnames}($fvals)) | |
| end |
can be replaced with
@inline function getproperties(obj)
fnames = propertynames(obj)
NamedTuple{fnames}(getproperty.(Ref(obj), fnames))
end? It infers just fine on 1.7+.
jw3126
Metadata
Metadata
Assignees
Labels
No labels