GLM.jl icon indicating copy to clipboard operation
GLM.jl copied to clipboard
verified publisher icon JuliaStats

Change all uses of wts to weights

Open andreasnoack opened this issue 1 year ago • 3 comments

Closes #350

I've also removed some outdated developer comments instead of updating names of functions that no longer exist. I'm opening this PR against the PR that fixes the doc build on Julia 1.11 to make tests pass. GitHub will automatically change the target branch to master once #569 has been merged.

andreasnoack avatar Nov 19 '24 15:11 andreasnoack

Thanks for this. Could you add a deprecation for the wts argument? Better limit breakage when it's easy to do.

nalimilan avatar Nov 21 '24 08:11 nalimilan

Also, if we want to release 2.0 quickly so that https://github.com/JuliaStats/GLM.jl/pull/487 may not be merged yet, it would be good to have this PR require weights to be FrequencyWeights, AnalyticalWeights or ProbabilityWeights, with an error message indicating to do weights=fweights(wts) to get the 1.x behavior. Otherwise we'll have to keep silently assuming that weights are frequencies, which often leads to mistakes (in R analytical weights are assumed).

nalimilan avatar Nov 21 '24 08:11 nalimilan

I think we should wait for #487. It seems really close. I'll update this PR once #487 has been merged.

andreasnoack avatar Nov 21 '24 10:11 andreasnoack