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

R packages #5645

Merged
merged 5 commits into from
Feb 3, 2020
Merged

R packages #5645

merged 5 commits into from
Feb 3, 2020

Conversation

Ivanluv
Copy link
Contributor

@Ivanluv Ivanluv commented Jan 29, 2020

@africanmathsinitiative/developers This is ready for review fixes #5601

@Ivanluv
Copy link
Contributor Author

Ivanluv commented Jan 29, 2020

@rdstern would you kindly look at this again

@rdstern
Copy link
Collaborator

rdstern commented Jan 29, 2020

ok Thanks

@rdstern
Copy link
Collaborator

rdstern commented Jan 29, 2020

Looked again and works very smoothly now - when it loads. It no longer asks for the mirror or the package.
When I make a mistake, e.g. try just putting a space into the package name - or (say) xxx, then the check says there is no package name. But OK is enabled. It doesn't give an error and the command line - in the output window is just the same.

I queried before whether it could still give the list of packages available - which it did before, even when the name was ok. This time I would like it to give that list if there is an error, so we can see what is available? Is that possible?

I would like it to do something different when it is successful compared to when it fails. Can it say anything about the loaded system in the output window? Whan you give the command in the script window that information is displayed in its own window, though not in the output.

@Ivanluv
Copy link
Contributor Author

Ivanluv commented Jan 31, 2020

install.package does not give an error when the package is wrong or does not exist.It however gives a warning which is capture correctly in Rstudio but not captured in the R Console.An option is upgrading a warning to o an error using option(warn=2) which adds an extra warning

@@ -0,0 +1,101 @@
Imports instat
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not needed

clsInstallPackage.SetRCommand("install.packages")
clsInstallPackage.AddParameter("repos", Chr(34) & "https://cran.rstudio.com/" & Chr(34))
ucrBase.clsRsyntax.SetBaseRFunction(clsInstallPackage)
End Sub
Copy link
Collaborator

Choose a reason for hiding this comment

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

A space after each sub.

Private Sub SetDefaults()
clsInstallPackage = New RFunction
clsInstallPackage.SetRCommand("install.packages")
clsInstallPackage.AddParameter("repos", Chr(34) & "https://cran.rstudio.com/" & Chr(34))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Parameter position?

TestOkEnabled()
End Sub

Private Sub check()
Copy link
Collaborator

Choose a reason for hiding this comment

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

capital "C"

cmdCheck.Enabled = False
ucrInputMessage.Enabled = False
End If
End Sub
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is possible to do this through our linking system.

SetRCodeForControls(bReset)
bReset = False
TestOkEnabled()
End Sub
Copy link
Collaborator

Choose a reason for hiding this comment

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

space

@maxwellfundi
Copy link
Contributor

@Ivanluv this needs a few changes to get it ready to merge. Could you get these few changes done soon?

@maxwellfundi
Copy link
Contributor

@rdstern Please review this.

@maxwellfundi
Copy link
Contributor

Tested this with @rdstern :-)

@maxwellfundi maxwellfundi merged commit 3378499 into IDEMSInternational:master Feb 3, 2020
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.

Adding a dialogue to install a new (or updated?) R package
4 participants