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

Predict dialogue#5127 #5537

Merged
merged 10 commits into from
Nov 29, 2019

Conversation

Ivanluv
Copy link
Contributor

@Ivanluv Ivanluv commented Oct 3, 2019

fixes #5127
adding the show argument check box.
@dannyparsons how are the iSetCursorBackCharacters determined in the function AddToReceiverAtCursorPosition which is in ucrReceiver expression

@Ivanluv
Copy link
Contributor Author

Ivanluv commented Oct 8, 2019

@africanmathsinitiative/developers can someone review this

@rdstern
Copy link
Collaborator

rdstern commented Oct 8, 2019

I like the look of the dialogue now.
I assume we could use any data from the extremes package to test. I checked by using the moorings data from the climatic library where I just then used climatic > prepare > extremes and got the maximum for the year. This produced a variable with one complication that it has missing values.

Then I used the Fit Model > Extremes and fevd, adding the argument na.action=na.omit to get the fitted model. Then I used the new Use Model dialogue.

a) It looks great now.
b) The dialogue name is given as dlgUseModel in the top left. Let's make it Use Model.
c) The show arguments seems to work fine.
d) Remembering the past commands used seems to work fine.
e) I suggest the Clear button be moved to the right-hand side of the dialogue and the same level as the keys on the general keyboard. There is space there.
f) The Try button needs to move up and a Save Result checkbox be added below the Try. It is the equivalent of the Save Model checkbox in the Model dialogue.
g) I first tried the ci (extremes keyboard) which works fine. I then tried another key and it didn't work. When it doesn't work the previous result is displayed in the output window. It should be cleared and not displayed if possible.
h) The Try option always seems to give the information that there is an error. This seems independent of whether there is one or not.

@rdstern
Copy link
Collaborator

rdstern commented Oct 14, 2019

This now all seems to sort of work and looks really good.
When @dannyparsons checks I hope he can look at the following "features".

I took an example datatset from the extRemes package (temperatures). Then used the Model > Model dialogue to fit an extreme value distribution.

Then the Model > Use Model dialogue.

I am back to having the same problem as before - which went away temporarily! When I try the plot, then the first time the plot appears very quickly and then is gone again. After this I don't get the plot?

The print and summary seem to work fine. But when I opt to save, then I no longer get any results in the output window. And I can't find the object I saved. I assume it isn't yet being added to the workbook?

@Ivanluv
Copy link
Contributor Author

Ivanluv commented Oct 15, 2019

@rdstern I've made some changes and I think saving this now works

Copy link
Member

@volloholic volloholic left a comment

Choose a reason for hiding this comment

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

@maxwellfundi @rdstern has been checking this and it now looks ok to merge once the changes to dlgModelling are removed, those are incorrect and should not belong here anyway.

@@ -255,7 +255,7 @@ Public Class dlgModelling
Private Sub cmdlm_Click(sender As Object, e As EventArgs) Handles cmdlm.Click
Clear()
If ucrChkIncludeArguments.Checked Then
ucrReceiverForTestColumn.AddToReceiverAtCursorPosition("lm(formula = , data, subset, weights, na.action,method = ""qr"", model = TRUE, x = FALSE, y = FALSE, qr = TRUE, singular.ok = TRUE, contrasts = NULL, offset)", 143)
ucrReceiverForTestColumn.AddToReceiverAtCursorPosition("lm(formula = , data, subset, weights, na.action,method = ""qr"", model = TRUE, x = FALSE, y = FALSE, qr = TRUE, singular.ok = TRUE, contrasts = NULL, offset)", 133)
Else
Copy link
Member

Choose a reason for hiding this comment

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

Why are there any changes here? This file is not relevant.

Furthermore when we checked this it was correct before at 143!

Copy link
Contributor

Choose a reason for hiding this comment

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

@Ivanluv could you change this minor issue here?

Copy link
Contributor

Choose a reason for hiding this comment

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

@volloholic @rdstern This is already resolved now.

@rdstern rdstern mentioned this pull request Nov 26, 2019
@rdstern
Copy link
Collaborator

rdstern commented Nov 29, 2019

Now that the segmented keyboard is included in the master it would be good if the issue raised by @volloholic could be resolved as well as the conflicts, so this could be merged.

@maxwellfundi maxwellfundi merged commit 965d34b into IDEMSInternational:master Nov 29, 2019
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 to the model dialogue 3 - a predict dialogue
4 participants