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.

Sign up for GitHub

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

Jump to bottom

Add notebook demonstrating proteomic aggregation and example analysis #120

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open

Add notebook demonstrating proteomic aggregation and example analysis #120

wants to merge 35 commits into from
+16,013 −3

Conversation

samobermiller
Copy link
Collaborator

@samobermiller samobermiller commented Jan 31, 2025 •
edited
Loading

#95

New Notebook Submissions:

  • Have you included a summary of the notebook in the README.md included updated links to the notebook?
  • Does your PR include links to the new notebook (in the branch) for review using nbviewer, Colab, and reviewnb? These three are the preferred ways to review changes and additions to notebooks during review.
  • Does your PR include a test in a github workflow that tests the render-ability of your notebook?

python:

https://colab.research.google.com/github/microbiomedata/nmdc_notebooks/blob/95-create-notebook-that-aggregates-proteomic-workflow-outputs-and-visualizes-the-results/proteomic_aggregation/python/proteomic_aggregation.ipynb

https://nbviewer.org/github/microbiomedata/nmdc_notebooks/blob/95-create-notebook-that-aggregates-proteomic-workflow-outputs-and-visualizes-the-results/proteomic_aggregation/python/proteomic_aggregation.ipynb

@samobermiller
added folder with new notebook proteomic_aggregation_and_visualizatio…
0cc9204 
…n, also added scipy and jupyter to requirements.txt (not sure if jupyter necessary or assumed as preexisting but had to recreate my venv so added it). changed pandas version to 2.1.2 because 2.1.1 was generating: 'ValueError: numpy.dtype size changed, may indicate binary incompatibility', when pandas was being imported in ipynb (checked and appears there was bug fix in 2.1.2)
@samobermiller
rerender of notebook
163d343
@samobermiller samobermiller linked an issue Jan 31, 2025 that may be closed by this pull request
Create notebook that aggregates proteomic workflow outputs and visualizes the results #95
Open
10 tasks
@review-notebook-app Review Notebook App
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@samobermiller samobermiller requested a review from bmeluch January 31, 2025 21:00
bmeluch
bmeluch reviewed Feb 1, 2025
View reviewed changes
@samobermiller samobermiller requested a review from kheal February 3, 2025 18:52
@samobermiller
copied nmdc_api and removed repeat functions from aggregation_functio…
b0784d9 
…ns.py. could not figure out way to import across different subfolders right now. also added table length limit of 6 for pandas df printouts and changed almost all cells to single output
@samobermiller
checks failed, retry push
add91ba
kheal
kheal reviewed Feb 4, 2025
View reviewed changes
@kheal
Copy link
Collaborator

kheal commented Feb 4, 2025

@samobermiller - some of my comments might be out of date, feel free to resolve them. I didn't realize I didn't "submit" them last night doh!

kheal
kheal reviewed Feb 4, 2025
View reviewed changes
@samobermiller
adjustments based on katherine comments, remaining bea comment and di…
71efd7b 
…scussion with lee ann. will still need to change in folder readme. this version includes t test and pvalues, may remove depending on further discussion
@samobermiller
Copy link
Collaborator Author

may remove current t test and pvalue analysis and replace with missingness figure

@samobermiller samobermiller requested a review from bmeluch February 6, 2025 01:34
@bmeluch bmeluch requested a review from lamccue February 10, 2025 17:26
lamccue
lamccue reviewed Feb 10, 2025
View reviewed changes
@samobermiller
adjustments based on Lee Anns comments. also renamed folder and scrip…
04889db 
…t to proteomic_aggregation since there is no longer analysis per say
@samobermiller
Merge branch 'main' into 95-create-notebook-that-aggregates-proteomic…
b9be746 
…-workflow-outputs-and-visualizes-the-results

other notebooks were updated to pass checks. previously failing because new data was added that didnt accommodate hardcoding
@samobermiller
remove some random files i accidentally pushed
4b5dcbb
@samobermiller samobermiller marked this pull request as ready for review February 19, 2025 22:42
bmeluch
bmeluch reviewed Feb 20, 2025
View reviewed changes
@bmeluch
Copy link
Contributor

bmeluch commented Feb 20, 2025 •
edited
Loading

I didn't add a comment for every single one but I would double check that all the markdown cells have periods at the end of their last sentences.

Colab runs and nbviewer looks good to me :)

@kheal
Copy link
Collaborator

kheal commented Feb 25, 2025

@samobermiller this has approved a lot, really nice job. I made some minor requests, but I think this is a great shape.

@kheal kheal self-requested a review February 25, 2025 01:26
CamiloPosso
CamiloPosso reviewed Feb 27, 2025
View reviewed changes
proteomic_aggregation/python/proteomic_aggregation.ipynb
@@ -0,0 +1,3582 @@
{
Copy link

@CamiloPosso CamiloPosso Feb 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Are the axes on a log-scale? I don't understand the zoomed in plot either. I think this plot could use a short explanation before it to guide the user and highlight the important pieces.


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This plot was using kernel density estimation so I could visualize smooth lines instead of a histogram. But after trying to expand on the explanation, I decided to rework the figure instead so it will just be a histogram of peptide spectrum matches

@bmeluch bmeluch self-requested a review February 28, 2025 19:34
@bmeluch bmeluch changed the title DO NOT MERGE: add notebook demonstrating proteomic aggregation and example analysis Add notebook demonstrating proteomic aggregation and example analysis Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lamccue lamccue lamccue left review comments

@CamiloPosso CamiloPosso CamiloPosso left review comments

@bmeluch bmeluch bmeluch approved these changes

@SamuelPurvine SamuelPurvine Awaiting requested review from SamuelPurvine

@pdpiehowski pdpiehowski Awaiting requested review from pdpiehowski

@kheal kheal Awaiting requested review from kheal

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create notebook that aggregates proteomic workflow outputs and visualizes the results
5 participants
@samobermiller @kheal @bmeluch @lamccue @CamiloPosso