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

Added logical operations into Prepare>Column: Numeric>Transform dialog #7238

Merged

Conversation

anastasia-mbithe
Copy link
Contributor

This fixes #7236
Hello @shadrackkibet, the controls are in place you can carry on.

@shadrackkibet shadrackkibet changed the title Additions to the Prepare<Column: Numeric< Transform dialog Added logical operations into Prepare>Column: Numeric>Transform dialog Feb 22, 2022
@shadrackkibet
Copy link
Collaborator

@anastasia-mbithe I have now added logical operations into this dialog. Could you first pull these changes locally and test them from your end? I did some quick testing as follows;

  • Loaded the famous survey data
  • Selected the first Numeric option
  • Switch to the logical radio button
  • Added field into the receiver
  • In the Input, I typed 3
  • Click OK

image

I also tried various logical options from the drop-down i.e >,<,<= etc
Also tried switching between different data frames and options at the top.

Results

  • No bugs were identified.
  • I got a logical column (filed1) added to the Survey data frame (snippet below)
    image

You are welcome to test it your own way then we will invite @rdstern to test it.

@anastasia-mbithe
Copy link
Contributor Author

@shadrackkibet the only issue now is reset again. After testing all the operations, I clicked on Sort button then clicked reset and went back to logical instead of round.
Screenshot (68)

I can fix that.

@shadrackkibet
Copy link
Collaborator

Please go ahead

@anastasia-mbithe
Copy link
Contributor Author

@shadrackkibet I fixed the reset issue and have also made changes to the test ok such that OK is not enabled when the ucrInputLogicalValues is empty for all operations except "is.na" and "!is.na"

@shadrackkibet
Copy link
Collaborator

Great. @anastasia-mbithe I would like you to illustrate in writing here how you tested it. Then I think we are ready to invite others to test.

@anastasia-mbithe
Copy link
Contributor Author

@shadrackkibet Okay.

  • So, I opened the survey data
  • then I opened the dialog and filled the receiver,
  • selected the Logical button and for all operations apart from "is.na" and "!is.na" test ok was disabled, when the input is empty
    Screenshot (73)

But for "is.na" and "!is.na" test ok is enabled
Screenshot (74)

For the other operations when I filled the input with 6, test OK was enabled
Screenshot (75)

  • I then did some different operations and then selected the Scale button,
  • Then multiplied the column in the receiver by 5
  • Then clicked reset and took me directly to the Round button.
    Screenshot (76)

@rdstern
Copy link
Collaborator

rdstern commented Feb 27, 2022

@anastasia-mbithe please check again. Maybe I have done something wrong, but it doesn't seem to work at all. I did this:
image

Notice the preview is not consistent with the logical function. Neither are the results. here they are:

field <- data_book$get_columns_from_data(data_name="survey", col_names="field", use_current_filter=FALSE)
field4 <- field
data_book$add_columns_to_data(data_name="survey", col_name="field4", col_data=field4, before=FALSE, adjacent_column="field")

It doesn't even give a logical function.

Please check each of the logical functions in turn, and also confirm that the preview updates correctly.

@anastasia-mbithe
Copy link
Contributor Author

@rdstern,The preview works fine now as well as the edit preview for any of the options. In the case of Logical, the operations update automatically in the preview but the values, it updates after clicking the Update button. But even without clicking Update button it still works to Script or OK.

Screenshot (79)

You can test it from your side now.

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@anastasia-mbithe great. From my point of view it all seems to be working now. Over to @shadrackkibet or @lloyddewit to check the code

@shadrackkibet shadrackkibet merged commit cf3ffa2 into IDEMSInternational:master Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A simple addition to the Prepare > Column: Numeric > Transform dialogue
3 participants