Revert to zero dependencies

[mcabbott]

Until recently this package had no dependencies at all. It defines rules for SpecialFunctions and bounds the version needed, but does not load the package. I see no reason that the same mechanism can't be used for LogExpFunctions.

I didn't notice this in #69, and I see no discussion of this change there, everyone was focused on other issues. I think that moving away from zero deps is something that we need a positive reason to do. Until we have one, keeping the old behaviour seems preferable.

Test failures for 1.7 aren't new, or are new only in that 1.7 is the new 1, not caused by this PR:

100
check rules: Test Failed at /home/runner/work/DiffRules.jl/DiffRules.jl/test/runtests.jl:54
101
  Expression: isapprox(dy, finitediff((z->begin
102
                Base.mod(foo, z)
103
            end), bar), rtol = 0.05)
104
   Evaluated: isapprox(-4.0, 33023.599675503; rtol = 0.05)
105
Stacktrace:

[codecov-commenter]

Codecov Report

Merging #76 (5b797c3) into master (b6301e8) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #76   +/-   ##
=======================================
  Coverage   95.74%   95.74%           
=======================================
  Files           2        2           
  Lines         141      141           
=======================================
  Hits          135      135           
  Misses          6        6           

Impacted Files	Coverage Δ
src/rules.jl	99.23% <100.00%> (ø)

---

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 b6301e8...5b797c3. Read the comment docs.

[devmotion]

I'm not against this PR but I want to point out that

Until recently this package had no dependencies at all.

is incorrect. The package depends on SpecialFunctions and NaNMath since Project.toml was added in 0.1.0: https://github.com/JuliaDiff/DiffRules.jl/blob/v0.1.0/Project.toml

Therefore I followed the same approach in the LogExpFunctions PR (which was already an indirect dependency through SpecialFunctions) and added LogExpFunctions as a direct dependency.

[mcabbott]

Yes, the quoted sentence is a bit ambiguous, but the next sentence explains. There are two meanings to dependencies, one is having things in Project.toml (which bounds versions and forces them to be downloaded) and the other is loading them (which needs to run all the packages). The second is strictly more brittle than the first, as packages which error on loading can cause things to fail.

In the first sense, the package does and did have dependencies, as you say.

This PR addresses only the second sense.

[devmotion]

the other is loading them (which needs to run all the packages). The second is strictly more brittle than the first, as packages which error on loading can cause things to fail.

If a package fails to load it means the compat entry is wrong and it should not be possible to install it in the first place, it seems? It should be possible to load all package versions that are stated to be compatible.

[cossio]

I agree with the approach in this PR. Having to manually check isdefined for each new function seems like re-implementing by hand what the compat bound is meant to accomplish in the first place.

If a package fails to load it means the compat entry is wrong and it should not be possible to install it in the first place, it seems? It should be possible to load all package versions that are stated to be compatible.

Not necessarily. There could be a bug in the dependency, which gets corrected in the next patch release (of the dependency). In this case, the compat entry here is correct.

[mcabbott]

Yes. That was the case with the (fairly) recent Static.jl apocalypse -- some internal macro it was using changed, and thus about 1000 packages could no longer load the its code, until this was fixed. But a package which simply downloaded its code would have been unaffected.

[cossio]

CI passes here. Merge?

[devmotion]

Looks good to me if tests pass (apart from the known issues).

Maybe you could apply one additional cosmetic change and move the rule to the list of binary rules, probably after xlogy which it corresponds to. Currently the rule is stated at the end of the file only because it is loaded conditionally.

[cossio]

Friendly bump. Can we merge this?