-
Notifications
You must be signed in to change notification settings - Fork 13
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
Make decisions up to consensus clearer #73
Comments
I completely agree. I'm new to this data set and not all steps are clear. Especially point (5) - in a way I would expect that normalisation is done by plate to correct for systematic differences between plates, at least that's what I am used to from other screens. |
Also, 'normalized by whole plate' and 'normalized by DMSO': does this refer to all samples from one plate vs. all DMSO controls from one plate or is it all samples from one plate vs. all the DMSO samples from all > 130 plates? |
Great to have these suggestions! And good to discuss here briefly before you invest time into making the actual fixes. I'll respond to these items one by one:
Do you think it would be helpful to add this detail to
I agree.
I think so! I forget why we didn't in the first place. I think @shntnu might know - we made these decisions largely to mirror the organization the IP decided a while ago.
Sounds good! I think we'll want to have some written form of this for the paper, eventually. @niranjchandrasekaran - what do you think? Right now we don't have too much explanation in
Ah, I was confused by this in #72 (comment) too - are you talking about the name of the spherized output files (e.g. |
This README.md currently points to |
sounds great. @michaelbornholdt do these descriptions help?
I recommend that you file a pull request that adds documentation to specifically address the points of confusion we discuss above. @niranjchandrasekaran or I can review the PR. I don't think it'll take long, and it will be very helpful to have at least one other person double-checking documentation for clarity. |
Ok, I can do that.
|
sounds good! Up to you if you want to also address #72 in this PR - having two PRs is sometimes better For the moving around of files, I think it makes sense. It's also very possible that I am missing/forgetting a critical detail of why we have it this way - @shntnu will need to double check this. There's no reason why you can't get started on that first paragraph and updating the flow diagram (LMK if you need pointers on this flow diagram) before hearing from Shantanu. |
Yes please, I have been on the doc where all the flow diagrams are but don't have the link anymore :) |
I'll send you the link via slack. Feel free to completely edit this document (I've saved a duplicate elsewhere). You can also feel free to depart from the pycytominer conventions and to create a pipeline diagram that specifically discusses our pipeline in this specific repo. I can use this figure in our paper in progress ;) |
I didn't follow. Can you clarify what the new folder structure would look like @michaelbornholdt |
The new structure would be | - profiles |
I'd favor keeping things within existing structures if possible; I made some edits to this: That would mean that the spherize script would live in a different folder from its output, but at least everything is together. What do you think? |
Nice! A couple comments and a decision point for you. Comments
Decision pointThe comments I made above are specific to the data in this repo. We might also decide to only include the data processing we used in the LINCS complementarity paper (see here for the exact data types we used). I might actually be in favor of this since we'll also be able to use it as a supplementary figure in that paper and it will be easier to make - although it might be misleading to only include some of the data versions we created. Maybe we can create both figures |
I think your comments and especially the new workflow figure makes things a lot clearer, thanks! I agree with most of Greg's comments. Additional points from my side:
General question: is there an accompanying manuscript somewhere? I assume it is going to be the analysis coming out of the LINCS profiling analysis. |
Now since the spherized profiles are a different level (kind of) than the other profiles, I am now convinced to keep them apart. As they are right now. @gwaygenomics |
yes!!! Love it. Couple of final comments:
|
Looks a lot better now! |
Continue discussion on: #75 |
Close since resulting PR was merged |
Talking to Mattias I noticed that there are some holes in terms of explaining what happens to get to the consensus data.
spherized_profiles
in the profiler folder, makes more sense I thinkSorry for all these suggestions at once. I don't what is on your plate (haha) right now @gwaygenomics so I'm unsure how to move forward with these suggestions.
CC: @FloHu, @shntnu @niranjchandrasekaran
The text was updated successfully, but these errors were encountered: