fix #685

[cossio]

Attempt to fix #685

[cossio]

Tests pass locally. CI failure seems unrelated to this PR, and is due to CI using deprecated actions. To fix that, please consider merging the Dependabot PRs.

[cossio]

Integration tests are also failing for #693, which suggests that is unrelated to the fix introduced in this PR.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.46%. Comparing base (134b9fd) to head (2cf28d0).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #688      +/-   ##
==========================================
+ Coverage   93.44%   93.46%   +0.01%     
==========================================
  Files          14       14              
  Lines         992      994       +2     
==========================================
+ Hits          927      929       +2     
  Misses         65       65              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[cossio]

Friendly bump.

[devmotion]

I think currently tests don't cover all changes in the PR: #688 (comment)

This seems to be confirmed by the code coverage analysis: https://app.codecov.io/gh/JuliaDiff/ChainRulesCore.jl/pull/688?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff

[oxinabox]

While it doesn't hit the one for AbstractThunk, the code is right.
And it is needed because of ambiguity.
But yes, it could be hit by writing a test that uses an InplaceThunk

[cossio]

I added a test:

it = InplaceableThunk(x -> x + [1], @thunk [1.0])
@test ProjectTo(BitArray([0]))(it) == NoTangent()

But that still gave an ambiguity error. So I replaced:

(::ProjectTo{NoTangent})(::AbstractThunk) = NoTangent()

with

(::ProjectTo{NoTangent})(::InplaceableThunk) = NoTangent()

which solves the issue.

And now both method definitions are hit by the tests.

[cossio]

Could we tag release with this? If so, should I bump the version in this PR?

[devmotion]

Yes, good point, please go ahead and update the version number in this PR.

[cossio]

please go ahead and update the version number in this PR.

Done.