Use mvrsquared instead of home-grown code#86
Open
Matt-Int wants to merge 14 commits intoTommyJones:mainfrom
Open
Use mvrsquared instead of home-grown code#86Matt-Int wants to merge 14 commits intoTommyJones:mainfrom
Matt-Int wants to merge 14 commits intoTommyJones:mainfrom
Conversation
As per issue TommyJones#84, this should switch out the homegrown r-squared code from textmineR package with the mvrsquared `calc_rsquared` function. Tested and got the exact same r-squared from the old version and the new one. I've kept all of the original input testing intact.
Also added some new lines to keep to 80 characters.
Was getting an error when running checks on Linux, adding this made the error no longer appear; may be because I didn't have Pandoc installed, but not sure.
Currently a bit slow, I suspect this is because of %*% and that mvrsquared potentially handles this in a c++ export, but I think this is fine as long as this isn't used directly in the CalcTopicModelR2 function.
Added the `...` as an argument to match the generic. The .Rd file is also automatically updated with this change, as is the NAMESPACE.
As they're no longer used in the package keeping them in here introduces some build errors.
This is what I get for programming past midnight...
Contributor
Author
|
I just realised that I need to pass the I'll do that tomorrow afternoon Europe time and test that things are still working. |
Removed some of the new lines which were making it a bit less readable. Also added the `...` arguments to the `calc_rsquared` function call. Unfortunately this means that previous calls to this function that used `cpus` will now be broken.
Changing the `threads` back to `cpus` as the vignettes failed to build and there were some additional unexpected behaviour from changing that argument name
Updated the parameter list and description following the previous commit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This should switch out the homegrown r-squared code from textmineR package with the mvrsquared
calc_rsquaredfunction. As per #84.Tested and got the exact same r-squared from the old version and the new one. I've kept all of the original input
testing intact.
What followed was just some troubleshooting to get everything building and testing on my machine; but it all builds and passes now!