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

remove (unused) named kwarg from ranef #469

Merged
merged 1 commit into from Feb 17, 2021
Merged

remove (unused) named kwarg from ranef #469

merged 1 commit into from Feb 17, 2021

Conversation

palday
Copy link
Member

@palday palday commented Feb 17, 2021

Closes #467

@palday palday requested a review from ararslan February 17, 2021 11:47
@codecov
Copy link

codecov bot commented Feb 17, 2021

Codecov Report

Merging #469 (3582552) into master (49d2cd5) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #469   +/-   ##
=======================================
  Coverage   92.82%   92.82%           
=======================================
  Files          23       23           
  Lines        1742     1742           
=======================================
  Hits         1617     1617           
  Misses        125      125           
Impacted Files Coverage Δ
src/linearmixedmodel.jl 94.27% <ø> (ø)

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 49d2cd5...3582552. Read the comment docs.

"""
function ranef(m::LinearMixedModel{T}; uscale = false, named = false) where {T}
function ranef(m::LinearMixedModel{T}; uscale = false) where {T}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function ranef(m::LinearMixedModel{T}; uscale = false) where {T}
function ranef(m::LinearMixedModel{T}; uscale = false, named = nothing) where {T}
if named !== nothing
Base.depwarn("the `named` keyword argument is deprecated; it has no effect", :ranef)
end

This avoids making a breaking change, since user code currently calling ranef(...; named=true) will error if the argument is removed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is queued for 4.0, which has other breaking changes.... but this would provide increased backwards compatibility (as most of the breaking changes are in the internal representation that the average user won't touch)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah if it's queued for a breaking release, might as well break it 😄

@palday palday merged commit e14078a into master Feb 17, 2021
@palday palday deleted the pa/ranefkwarg branch February 17, 2021 20:06
palday added a commit that referenced this pull request Apr 12, 2021
palday added a commit that referenced this pull request Apr 12, 2021
* remove (unused) `named` kwarg from ranef (#469)

(cherry picked from commit e14078a)

* deprecate unused `named` kwarg

* news + patch bump
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.

Drop kwarg named from ranef
2 participants