RobertsLab / project-oyster-oa

2 stars 3 forks source link

L's feedback to date #19

Closed laurahspencer closed 7 years ago

laurahspencer commented 7 years ago

Well done! You use lots of tricks that streamline scripts, which I will try to incorporate into my scripts. Here are my observations/suggestions:

1) Transitions list link in jupyter notebook is broken:

image

4) This might be alarming to someone not used to Skyline, could add a note to protocol:

y-test3

3) You could reduce your .fasta used in the SRM Skyline doc to only include target proteins. As is, there isn't a step in the protocol where you remove the proteins (from targets pane) not targeted in SRM, so when importing .raw data it takes much longer than it would with just the target peptides populating the analyte tree, and without going through each protein to check for chromatograms I don't know which proteins are my target (maybe you know a Skyline trick to unfold only proteins w/ peaks???)

image

4) You could use read.csv(url("http...")) in your R scripts to directly pull data from your repo

5) Step 7b in the protocol has me duplicate/edit the sequence file; could instruct user to use this edited file in your R script:

image

6) Error merging biological data to abundance data; possibly b/c the column name in biol. data set isn't exactly "Sample.Number"

image

After changing that column's name to "Sample.Number" I executed the merge, but dataframe is empty; I think it's b/c your X db doesn't include sample numbers w/o replicates:

image image

7) "Your Sample Key"/"Sequence File" doesn't have any TIC values:

image

8) You could add install.packages("reshape2") (etc.) for any non-baseR packages used (although I'm curious if we could write install code that would only "run if" it wasn't already installed....

... the merge error (detailed in 6) is causing problems for the rest of the code, so I'll wait until that's fixed to continue working through the scripts.

yaaminiv commented 7 years ago

Thanks for the feedback! Addressing 6)....

...I actually didn't get any errors. I was able to run the script with no problems after opening it (nothing saved in my global workspace).

screen shot 2017-10-11 at 11 19 16 pm

I'm assuming you already tried clearing your workspace and running through the code again?

yaaminiv commented 7 years ago

1: Link is not broken! Will take you here: https://github.com/RobertsLab/project-oyster-oa/blob/master/analyses/DNR_TransitionSelection_20170707/2017-07-08-Final-Transitions/2017-07-10-SRM-Transitions-With-PRTC.csv 3: Using the linked file above when populating the analyte tree resolves this issue (procedure outlined in Step 2d) 6: See above comment 7: There are two separate files you are referring to here. The "Key for Samples" in the notebook is the file straight off the machine, and it's an .xlsx document. The other sequence file you're referring to is a .csv used in my R script that includes TIC values and only has information for samples used in downstream analyses. I could definitely do a better job at highlighting the differences in Step 1 when I have you gather the materials. 8: Aside from reshape2 I don't believe I'm using any packages outside of base R (are there some I'm not aware of?!). Having install.packages in our code wouldn't affect a user if they already had that package installed. It would just overwrite what already exists. The real question is how to specify the package versions we're using.

I'll take the rest of your feedback and incorporate it into my wording at an hour that doesn't cause me to go cross-eyed 😆

yaaminiv commented 7 years ago

See #20, #21, #22, #23, #24, and #25 for further progress.