Distributions.jl
Distributions.jl copied to clipboard
JuliaStats
**BREAKING** Fix loss of structure in cov() and invcov() of AbstractMvNormal subtypes
Resolves #572. Distributions.jl has a lower bound of PDMats=0.10, in which PDMat and PDiagMat are subtypes of AbstractMatrix, so no need to call Matrix() anymore.
What, if anything, do I still need to add to this PR to get it merged ?
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Comparison is base (
7e232ca) 86.00% compared to head (f919388) 83.88%.
Additional details and impacted files
@@ Coverage Diff @@
## master #1373 +/- ##
==========================================
- Coverage 86.00% 83.88% -2.13%
==========================================
Files 144 144
Lines 8646 7004 -1642
==========================================
- Hits 7436 5875 -1561
+ Misses 1210 1129 -81
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
IMO this is a breaking change and therefore we should make a breaking release. Since people dislike breaking releases of Distributions (https://github.com/JuliaStats/Distributions.jl/issues/1317), we should check if there are any other breaking changes that could be included in Distributions 0.26.
I wonder if it is possible to check if/how breaking this would be for downstream packages. Is there actually any package that relies on cov returning a Matrix? I would like to merge this PR but it's blocked since it's technically breaking. In my experience though (also with e.g. SpecialFunctions 2) for such central packages such a tiny breaking change means putting a lot of work on downstream developers and requiring manual compat updates in many packages, but if the breaking change only affects very few packages it might not be worth it and easier to make a non-breaking release and fix a handful of packages instead of > 200.