What if lifted_getfield on a tangent just called getproperty
#39
Conversation
|
@Keno Any objections? This should be fine, right? |
|
If it works, ok by me. |
|
I plan to set up some CI as soon as I figure out the problem on latest Julia nightly. Since this doesn't seem urgent, I'll probably hold off on this until then, so we can merge with some confidence. |
simeonschaub
changed the title
getpropertylifted_getfield on a tangent just called getproperty
|
Ah, this breaks down for |
|
We should consider adding that overload in CRC so this works. |
also fixes some issues with `getindex`, mainly that it didn't canonicalize tangents with NamedTuple backing, which I'd argue is a bug. It's also more consistent with `getproperty` now, so `tangent[:a]` returns `ZeroTangent()` instead of erroring if `tangent`'s backing doesn't have a field `a`. ref JuliaDiff/Diffractor.jl#39
also fixes some issues with `getindex`, mainly that it didn't canonicalize tangents with NamedTuple backing, which I'd argue is a bug. It's also more consistent with `getproperty` now, so `tangent[:a]` returns `ZeroTangent()` instead of erroring if `tangent`'s backing doesn't have a field `a`. ref JuliaDiff/Diffractor.jl#39
simeonschaub
force-pushed
the
ox/lessweird_lft
branch
from
b321e6f to
4b2bc2f
Compare
|
@Keno Any ideas about the hang on Windows? Is that worth investigating? (Failing job is https://github.com/JuliaDiff/Diffractor.jl/runs/3658202821?check_suite_focus=true for reference.) |
|
No idea. I've never tried it on windows. I'd say just disable the CI job for now. |
Semantically these are the same.
but lets see if this works out right