IDEMSInternational / R-Instat

A statistics software package powered by R
http://r-instat.org/
GNU General Public License v3.0
38 stars 103 forks source link

Convert To Logical #4123

Open Patowhiz opened 7 years ago

Patowhiz commented 7 years ago

@africanmathsinitiative/developers When I try to convert factors or columns that have decimal numerics to logical an error is thrown. This error is being caught but the feedback doesn't look user friendly and easily understanding. I recommend an alternative feedback that a user can understand quite if the operation is unsuccessful or the conversion is impossible. error2

dannyparsons commented 7 years ago

Can you suggest something that's more helpful? We don't have complete control over error messages as some parts come direct from R but we can choose how it's displayed and some parts of text if we can think how to make it clearer to users.

Patowhiz commented 7 years ago

Well I have traced where the error comes from. Well, this is a bit tricky. I can see class clsRlink is heavily used by other dialogues but its subroutines don't have a way of returning the error raised. My experience shows me that he best place to display an error is not in the clsRlink but rather in the sub of the class that initiates the command. In this case, this is an error that we expect R link to throw when a user tries to convert a column that is not whole numbers to logical.

There's 2 ways in which we can solve this : 1) We could either check to see of the column has only whole numbers and if not display to the user a good message without executing the RCommand for converting to logical or 2) We could change the implementation of frmMain.clsRLink.RunScript() and make it update an error variable instead of itself displaying the errors. Then after its executed we could check the contents of the variable and act accordingly. This way we will ensure that even in future dialogues will show a well structured error messages depending on their functionality.

@dannyparsons I can correctly show this in code if you allow me to. I could create a new branch for this and you see how I solve this problem then it can be replicated by others. I will have to make changes in class clsRLinkand ucrDataView. The changes in clsRLink will not affect how subroutine RunScript() of line 359 is being used. This will help in showing how we can have control over error messages caught clsRLink

dannyparsons commented 7 years ago
  1. is definitely possible and would allow us to display a slightly nicer error message.

  2. I don't want to move the error displaying out of the RLink and into dialogs - otherwise we have to implement something in every dialog and lose the consistency across the project. The error message passed from R to the RLink is the only information we have about the error. It would be nice if the dialog could interpret this and display a better error but I don't see how that's possible since it's just plain text coming from R. Is that what you were suggesting or something different?

I'm happy to think about restructuring the format of those error boxes if we can get a structure that's more understandable. The first block is direct from R and the second is something we constructed. I agree it's not very friendly so open to suggestions for improvements.

Patowhiz commented 7 years ago

I'll show you how we can implement 2. without losing consistency in code. Let me finish the translation task.

dannyparsons commented 7 years ago

If you can do get an example quickly great, otherwise we should discuss so I understand better before you spend much time on it.