Complete peer review assignment

[kdestasio]

Strengths:

• Nice use of map and tidyverse functions.
• This is a good example of what it would look like to visualize real world data. I could see this coming in handy during a research project.
• The project incorporates much of the information learned in class. E.g. {glue}, nesting, {purrr}, custom functions, {here}.

I changed the code in the following ways:

• Took out calls to path(), which were redundant with the here() function.
• Condensed functions to use single call to glue() and again removed the path() functions
• Used modify_at() to change variables to integers, rather than individual calls to as.integer() for each one.
• Specify kable() digits for readability of the table.
• Removed flavor_id <- “” as it does not add anything to the code.
• Combine the two separate knitr::opts_chunk$set calls.

Areas for improvement:

• Suggest to make the code in the chunk labeled “ label-category” a function or loop. This could be a way to incorporate map2() or pmap().
• It would be helpful if there was a text box in the dashboard that explains the plot and table. Along those lines, a table title and plot captions would be useful.

[kdestasio]

Tagging @datalorax

[kdestasio]

Addressing the conflicts:

(1) Re. knitr::opts_chink$set() "separate in different lines to keep things clear." If you want this on two lines, you still don't need to call the function twice. You can instead do:

knitr::opts_chunk$set(message = FALSE, warning = FALSE, 
                      dpi = 300)

This makes it more visibly accessible to the reader as compared to.

knitr::opts_chunk$set(message = FALSE, warning = FALSE)
knitr::opts_chunk$set(dpi = 300)

(2) Addressing "through the whole scripts we are using path in case someone are going to run the codes over different operation system"

• the here() function fills in the home directory and path separators that are appropriate for the operating system it is used with. So, path() is not necessary when using here()

(3) ok

Edit: I've updated the code in the pull request to reflect my comments and to provide a tangible example of how it would work.

[Irisfee]

Hi @kdestasio,

Thanks for reviewing our codes and all the helpful comments!

That's a good point that not to call the opts_chunk twice and just separate the arguments in two lines!

for the here/path choice, it is really impressive that here can actually solve the problem and thanks for showing us the codes! However, I prefer path since it simplifies the arguments I need to put in, and seems to be easier to code (maybe just for me).

all in all, thanks for all your valuable suggestions!

Thanks, Xi & Yufei

[kdestasio]

That makes sense. Whatever is clearest for you is ultimately best :)

[datalorax]

Really nice review, Krista!

With regards to here::here, you actually can (and should) use exactly the same structure with here that you are currently using with path, so I agree with Krista that they are redundant. For example, you currently have

bids_dir <- path(here())         
deriv_dir <- path(bids_dir, "derivatives")
data_dir <- path(deriv_dir, "pattern_similarity")   

but this could be reduced to

here("derivatives", "pattern_similarity")

and the output should be identical.

This means when reading in data, it could simplify to

subj_list <- read_tsv(here("derivatives",  "pattern_similarity",  "participants.tsv"), 
                      col_types = "cicc")

And all the path stuff above that is currently in the code chunk titled dir-setting could go away.

Really good suggestions for improvement too!

[Irisfee]

Hi @datalorax ,

thanks for explaining the usage of here in detail!

However, I prefer to use path to solve the problem within the chunk "read-data", so I want to keep everything in a consistent way.

I prefer to have the dir-setting chunk since the data used in this assignment is just a small part of my actually dataset. It actually needs to read in more files from different folders under a standardized format of fmri dataset and save files in different folders. With the objects of the path to each folder just help me feel clearer to understand the structure of the data. Hope this can be just regarded as my personal coding style.

I do appreciate you all for introducing the function of here to us!

thanks Yufei

[datalorax]

Hi Yufei,

For the assignment, I'm fine with whatever you choose. But I do think that here alone is going to be the more efficient and clearer approach regardless of your data structure, and it is particularly important for exactly the situation you describe, when you have complex data stored across multiple directories. It will also make the code more robust and reproducible. So while you are free to make any choice, I would highly encourage you to think about this critically. I'd be happy to look at your actual project with you and discuss how I would use here within it if you'd like.