Irisfee / r3final

dashboard:
https://irisfee.github.io/r3final/
1 stars 3 forks source link

Grade and feedback #5

Open datalorax opened 5 years ago

datalorax commented 5 years ago

Thanks Yufei and Xi! This is really excellent work. The final product is clean and professional. Below are the grading components, followed by some feedback.

Grading

Feedback

simi_rtv_files <- dir_ls(here("derivatives"), 
                         recurse = TRUE,
                         regexp = "(?=.*retrieval)(?=.*zr_sim)(?=.*7-tmaps)",
                         perl = TRUE)

labels_files <- dir_ls(here("derivatives"), 
                       recurse = TRUE,
                       regexp = "(?=.*beh_)(?=.*pairwise)",
                       perl = TRUE)

simi_rtv_raw2 <- map2_df(simi_rtv_files, labels_files, read_simi)

identical(simi_rtv_raw, simi_rtv_raw2)
# [1] TRUE

This is a little complicated because of the regex but basically it's just an AND operator for different patterns you want to match. The regexp argument is just passed to grep, and so perl is passed to that, which is basically just a little more powerful regex operator. In your specific situation, you might need to swap out the actual pattern for whatever makes sense, and ideally you'd pull that programmatically from your python output. But the above would basically replace all of the lines up to 116 (minus the creation of your functions).

Rather than

simi_rtv_non_within_pair %>%
  mutate_at(vars(starts_with("i")), as.integer)

You can just add the additional argument convert = TRUE to separate. For example

separate(col = pair_match, into = c("i1_item", "i2_item"), 
         sep = 2,
         convert = TRUE) %>% 
separate(col = pair_match_obj, into = c("i1_obj", "i2_obj"), 
         sep = 2,
         convert = TRUE)

This will get you the same result.

i1_item i2_item simi_cond
1 1 within_item
1 2 same_color
1 3 within_pair
2 2 within_item
2 4 within_pair
.. .. ..
20 22 between_pair

Then when you joined the data, you would get the new simi_cond variable, as defined by their match with i1_item and i2_item. Entering data in a spreadsheet like this is less error prone, and merging two datasets is generally a great method for recoding data. Given your comment, it appears you were a bit concerned about this part. Had you of asked me about it prior to turning in your final project I likely would have directed you to this method and you would not have lost any points (although I only removed three points so it's not that big of a deal).

dat_rtv <- simi_rtv %>%
  group_by(group, subj_id, atlas_id, roi_id, roi_hemi, simi_cond) %>%
  summarise(similarity = mean(similarity)) %>%
  ungroup() %>% 
  filter(!simi_cond == 'other') %>% 
  filter(roi_id %in% c("ANG", "IPS","SPL","HPC","OTC")) %>%
  mutate(group = fct_recode(factor(group), "Paired" = "1", "Control" = "2"),
         simi_cond = factor(simi_cond, 
                            levels = c("within_item", 
                                       "same_color", 
                                       "between_pair", 
                                       "within_pair")))

Also, as a general rule, I'd try to keep your code within 80 characters as a maximum width. You can setup rulers in RStudio to help you with this. That will also ensure if you need to render to PDF that it will all be there and not run off the page (this is violated on line 245).

Irisfee commented 5 years ago

Hi Daniel,

THANK you so much for all the comments! I learned a lot through it. I will definitely check out all the points you have mentioned!

thanks Yufei