Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle RuleConfig and move to new world #4

Merged
merged 6 commits into from
Jun 11, 2021
Merged

Handle RuleConfig and move to new world #4

merged 6 commits into from
Jun 11, 2021

Conversation

oxinabox
Copy link
Member

This is the companion to JuliaDiff/ChainRulesCore.jl#363
which was breaking for this, but only in a silly way.

This PR does 3 things.

1 . corrects the incorrect definition of the frule fallback

That ChainRulesCore PR as a complete side change corrected the fallback from frule(f, args...) to frule(dargs, f, args...).
Which never really mattered since everything was Any typed, but still was wrong, and turned on CROG (ChainRulesOverloadGeneration) depended on that

2. drop support for CRC (ChainRulesCore) 0.9 and remove deprecated things

We needed to drop that so we can talk about RuleConfigs for next part

3. Filter out rules that require RuleConfig

Technically we can ignore these, adding the config wasn't actually breaking.
But it does make the hook function spew out a ton of warnings.
So we would rather not.

In some future PR we will bring them back.
but will need to think a bit about the API.
The user already does a lot of filtering, so probably we will want to let them do this filtering also.

@codecov-commenter
Copy link

codecov-commenter commented Jun 11, 2021

Codecov Report

Merging #4 (03b8233) into master (9bf7935) will decrease coverage by 1.70%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #4      +/-   ##
==========================================
- Coverage   98.00%   96.29%   -1.71%     
==========================================
  Files           2        2              
  Lines          50       54       +4     
==========================================
+ Hits           49       52       +3     
- Misses          1        2       +1     
Impacted Files Coverage Δ
src/ruleset_loading.jl 96.07% <100.00%> (-1.80%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9bf7935...03b8233. Read the comment docs.

@oxinabox oxinabox merged commit f73a842 into master Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants