Added delete of value labels to the Prepare > Column: Factor > View Levels/Labels dialogue using delete key in column metadata

[N-thony]

Fixes (partially) #7073
This is still a work in progress.
@dannyparsons I need your help here. Could we have a call? Thanks

[N-thony]

Fixes (partially) #7073 This is still a work in progress. @dannyparsons I need your help here. Could we have a call? Thanks

@dannyparsons could we have a discussion on this?

[rdstern]

@N-thony that seems fine, as far as it goes, until you have discussed with @dannyparsons Keep asking him!
a) I notice that Ok isn't enabled for the data frame, when you first use the delete option.
b) If you go to the columns, and choose a variable, and return to the data frame, then it is enabled. But it currently delivers the same command for the columns, not anything for the whole data frame. But that will change anyway.
c) Related, if easy, will be to delete a label from the metadata. I notice that this works in principle, but not in practice. It delivers the following command:

# Edited variables metadata value
data_book$append_to_variables_metadata(data_name ="survey",col_names = "fert",property="labels",new_val="")

Isn't that the sort of thing you want as your function. I also tried with:

data_book$append_to_variables_metadata(data_name ="survey",col_names = "fert",property="labels",new_val="NA")
Maybe you could be trying that as the code in your dialogue.
By the way it didn't seem to work, through the metadata window. Perhaps it will work for you, or that's what you could investigate while waiting for Danny?

Once sorted, then we should also see what needs to change in the metadata window. Currently it just seems to mess up the factor!

[N-thony]

@N-thony that seems fine, as far as it goes, until you have discussed with @dannyparsons Keep asking him! a) I notice that Ok isn't enabled for the data frame, when you first use the delete option. b) If you go to the columns, and choose a variable, and return to the data frame, then it is enabled. But it currently delivers the same command for the columns, not anything for the whole data frame. But that will change anyway. c) Related, if easy, will be to delete a label from the metadata. I notice that this works in principle, but not in practice. It delivers the following command:

# Edited variables metadata value
data_book$append_to_variables_metadata(data_name ="survey",col_names = "fert",property="labels",new_val="")

Isn't that the sort of thing you want as your function. I also tried with:

data_book$append_to_variables_metadata(data_name ="survey",col_names = "fert",property="labels",new_val="NA") Maybe you could be trying that as the code in your dialogue. By the way it didn't seem to work, through the metadata window. Perhaps it will work for you, or that's what you could investigate while waiting for Danny?

Once sorted, then we should also see what needs to change in the metadata window. Currently it just seems to mess up the factor!
@rdstern thanks for review though this is pending and will be ready for review after the help from Danny.
@dannyparsons could we discuss this or maybe guide me probably on the backend side? Thanks.

[N-thony]

@rdstern @shadrackkibet I have added the delete labels feature on View Levels/Labels dialogue, and in addition I have added the possibility of deleting the labels from Delete keyboard. Please take a look. Thanks.

[rdstern]

@N-thony I don't know if you checked the results of what you have done with the delete?
I first tried deleting all in the survey data. Then I found that the levels/Labels dialogue doesn't work at all. And the Labels has = NA for all variables, while perhaps it should be NA.
When I started again and deleted just the fert (which I had made a labelled factor, Then I was still ok examining the other factors with the Levels/Labels, but not the fert that I had deleted.
When choosing the variables I wonder if the data selector could just have the variables that have value labels to delete? (Perhaps ideally where there are none, there might be a message saying No Variables have Value Labels to Delete.

[rdstern]

I also notice that when there are no labelled variables, then that item is absent in the metadata. For example, look at the survey data when imported. There is no Value Label entry. If you then make fert into a factor, then it has value labels and the entry is provided. I suggest if you delete value labels from the data frame, then it might delete the entry completely, rather than keeping it with NA in every variable.

[N-thony]

@N-thony I don't know if you checked the results of what you have done with the delete? I first tried deleting all in the survey data. Then I found that the levels/Labels dialogue doesn't work at all. And the Labels has = NA for all variables, while perhaps it should be NA. When I started again and deleted just the fert (which I had made a labelled factor, Then I was still ok examining the other factors with the Levels/Labels, but not the fert that I had deleted. When choosing the variables I wonder if the data selector could just have the variables that have value labels to delete? (Perhaps ideally where there are none, there might be a message saying No Variables have Value Labels to Delete.

@rdstern of course, I did the test and it is very trivial to notice that, I was wondering why it was adding = even if you try to delete the label manually on the grid by replacing with nothing or NA it will change it to = NA, I had the check on that with @shadrackkibet and I was thinking that the R function need to be improved

[N-thony]

@rdstern @shadrackkibet I have resolved the comment above and made a small change in the used R function here.
@rdstern could you re-test? Thanks.
@shadrackkibet could you also test and check in the R function? Thanks

[N-thony]

@lloyddewit @rdstern I resolved and commented your reviews above. Please test/check if you are happy with? Thanks

[rdstern]

I am still happy and relieved to approve this. If possible I would still like the selector to indicate and just show the Labelled Variables. But that may come later

[lloyddewit]

@lloyddewit @rdstern I resolved and commented your reviews above. Please test/check if you are happy with? Thanks

@N-thony there are many comments above still unresolved. I started going through them and stating '@N-thony this still needs resolving' buth there are too many. Please go through all the comments again (some may be hidden) and ensure that you've pushed all your changes, thanks

[N-thony]

@lloyddewit @rdstern I resolved and commented your reviews above. Please test/check if you are happy with? Thanks

@N-thony there are many comments above still unresolved. I started going through them and stating '@N-thony this still needs resolving' buth there are too many. Please go through all the comments again (some may be hidden) and ensure that you've pushed all your changes, thanks

I think all is resolved now.

[lloyddewit]

@N-thony Thank you for the changes. there are 3 comments that still need to be resolved. If you make those changes, then I can approve, thanks

[lloyddewit]

@N-thony all my comments are now resolved, thanks

[lloyddewit]

@rdstern Please could you test again?
@shadrackkibet Would you also like to test/review this PR or may we go ahead and merge?
Thanks

[lloyddewit]

I am still happy and relieved to approve this. If possible I would still like the selector to indicate and just show the Labelled Variables. But that may come later

@N-thony Will you implement this in a separate PR?

[shadrackkibet]

@lloyddewit we just had discussions about this. Just a few things to sort out then we can merge.

[shadrackkibet]

@lloyddewit over to you.

[lloyddewit]

@rdstern If you're able to retest and approve then we can merge, thanks

[rdstern]

Great