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

Ensured that columns in the Data Grid can be deleted even when there are no key columns #8416

Closed

Conversation

lilyclements
Copy link
Contributor

Fixes #8413

@Vitalis95 can you review this, and then it can be ready for @rdstern to review

rdstern
rdstern previously approved these changes Jul 3, 2023
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.

@lilyclements many thanks - this seems to work fine now. @lloyddewit over to you

@lloyddewit lloyddewit changed the title Deleting Columns to not return an error if there is no key Ensured that columns in the Data Grid can be deleted even when there are no key columns Jul 4, 2023
@lloyddewit lloyddewit added the bug label Jul 4, 2023
Copy link
Contributor

@lloyddewit lloyddewit left a comment

Choose a reason for hiding this comment

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

@lilyclements Thanks looks good to me. If you can fix the indentation, then I can approve and merge

Comment on lines 774 to 781
if (!is.null(self$get_variables_metadata()$Is_Key)){
if (self$get_variables_metadata() %>% dplyr::filter(Name == col_name) %>% dplyr::pull(Is_Key)){
active_keys <- self$get_keys()
keys_to_delete <- which(grepl(curr_col_name, active_keys))
keys_to_delete <- which(grepl(col_name, active_keys))
keys_to_delete <- purrr::map_chr(.x = keys_to_delete, .f = ~names(active_keys[.x]))
purrr::map(.x = keys_to_delete, .f = ~self$remove_key(key_name = names(active_keys[.x])))
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

indents correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They should be! I just copied and pasted the file into R and then back into the VB document to autofix the indentations of everything in this file

Copy link
Contributor

Choose a reason for hiding this comment

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

@lilyclements I assume that you pasted the whole file into the RStudio editor, and then pasted back into the Visual Studio editor. Unfortunately the GitHub diff interprets this as all 4000+ lines changing (probably because of different white space characters and/or line breaks). This makes it impossible to see the real changes in the PR and has also triggered a merge conflict.

I suggest that you go back to the previous version of the file and fix just the indents mentioned above manually.

Note: you can get the previous version of the file by checking it out from commit 102436e. If you're unsure how to do this, then just let me know.

I'm sorry my comment caused such trouble, I only intended some spaces to be added to the few lines above.
Thanks for your patience.

image

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.

Delete columns sometimes doesn't work anymore
3 participants