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

Adding methods to get fitted, residual, and new predictions from lava… #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jebyrnes
Copy link

Adding methods to get fitted, residual, and new predictions from lavaan, pending them being added to the main package. I had a hard time compiling the package with devtools given a few of the scripts. Perhaps could you put a package build file in the master branch? If this all goes through, I'll add my lavSpatialCorrect script back shortly.

…an, pending them being added to the main package.
@TDJorgensen
Copy link
Member

Hi Jarrett,

Thanks for the contribution! I really think this is something important missing from lavaan, and clearly there are many who would like this functionality. I just saw David Kaplan post about this on the lavaan board, and I reminded me I had not gotten back to you yet. I am slammed with teaching and won't have time to actively maintain semTools until February (lots of changes to make then!), but I linked him to your source code on your github fork.

In the meantime, I have added a source tarball to the /builds subdirectory as you requested. Hope it helps.

Terrence

@TDJorgensen
Copy link
Member

Hi Jarrett,

I'm still hoping to have time to include this in the next version of semTools. Hopefully when lavaan goes to CRAN in about a month.

Can you check whether your code still works with the latest development version of lavaan?

Also, the NAMESPACE file seems to indicate we are importing a stats() function from the lavaan package, but there is none. This is because the following roxygen2 comment appears above each function:

#' @importFrom lavaan stats

I think you intend to import particular functions from these packages instead. Can you list them explicitly and update the pull request?

#' @importFrom lavaan func1 func2
#' @importFrom stats func1 func2

Thanks!

@drakesiard
Copy link

Hello,
I'm not sure of the etiquette involved, so please let me know if I'm doing something wrong by jumping in. I've rebased @jebyrnes's patch onto the most recent master and fixed up the roxygen2 comments (and function imports more generally); it's available here. I've also added a separate commit with some lint picking and general tidy up of examples, but that can be a separate PR if you'd like.

Would you like me to create a PR from my branch onto simsem:master, or onto jebyrnes:master? Also, please let me know if there is anything else you would need to make my changes package-worthy.

Thanks!

@TDJorgensen
Copy link
Member

Thanks for picking this up again, Drake. I would just accept a pull request from your own clone with the additional commit(s), and add you both to the DESCRIPTION file. I just had a baby, so my time is obviously limited, but before accepting a PR, I would want to make sure that informative errors are issued for situations (possibly) not accommodated by these functions, and check whether the output is consistent under different lavInspect(fit, "options").

  • categorical endogenous variables (or are they? If so, are expected probabilities or probits returned, and could this be controlled by a type = c("link", "response") argument like predict.glm() has?)
  • multilevel models (check using lavInspect(fit, "nlevels") == 1L)
  • works whether fixed.x=TRUE (default) or FALSE
  • works when conditional.x=TRUE (default in models with categorical endogenous variables)?

I think the calculations would be more complicated when conditional.x=TRUE, so no need to implement these methods for that option; just a check to warn users that they should set it FALSE in order to use the methods.

Finally, perhaps the function names could be extended to fitted_ov_lavaan() and such, to make sure users understand these functions are designed only for observed-variable models.

@drakesiard
Copy link

Thanks for the feedback. I'll look to make those changes and submit a new PR when I'm done.

p.s. Congratulations, by the way!

@TDJorgensen
Copy link
Member

Hi Drake, I just made some updates, some of which are related to R 4.0, so please make sure you are working with that version when you test your new PR (also use the latest lavaan 0.6-6).

FYI, I am about to submit a new semTools to CRAN. No rush on your new PR; I can include it in the next version.

@drakesiard
Copy link

Thanks for letting me know. Exam season is here, so I probably won't be able to update and finish the changes before mid-June.

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