biocore / metagenomics_pooling_notebook

Jupyter notebooks to assist with sample processing
MIT License
8 stars 16 forks source link

re Issue #23: adds fields for library_construction_protocol and experiment_design_description #36

Closed ahdilmore closed 2 years ago

ahdilmore commented 2 years ago

As outlined in in #23,

I haven't added any required values yet. Please let me know if there are any suggestions on required or suggested values.

Thanks, @ElDeveloper :)

ElDeveloper commented 2 years ago

Thanks so much!!

On Sep 23, 2021, at 10:36 AM, Hazel Dilmore @.***> wrote:

As outlined in in #23,

• Library Construction Protocol and Experiment Design Description are required fields for sample sheet • These two new required fields are added into the outlines in each notebook where sample sheets are created • metapool.prep.preparations_for_run fetches values from sample sheet I haven't added any required values yet. Please let me know if there are any suggestions on required or suggested values.

Thanks, @ElDeveloper :)

You can view, comment on, or merge this pull request online at:

https://github.com/biocore/metagenomics_pooling_notebook/pull/36

Commit Summary

• added library construction protocol and experimental design description as requirements • Merge branch 'biocore:master' into master • added fields for library construction protocol and experimental design in sample sheet metadata • fetches library_construction_protocol and experiment_design_description values from sheet instead of placeholders • experiment not experimental • remove TODOs File Changes

• M amplicon-pooling.ipynb (58) • M metagenomics_pipeline_MiniPiconorm.ipynb (6) • M metagenomics_pipeline_qPCRnorm.ipynb (19) • M metagenomics_samplesheet_generator.ipynb (19) • M metapool/prep.py (8) • M metapool/sample_sheet.py (16) Patch Links:

https://github.com/biocore/metagenomics_pooling_notebook/pull/36.patchhttps://github.com/biocore/metagenomics_pooling_notebook/pull/36.diff — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

ElDeveloper commented 2 years ago

@ahdilmore, looks like we should be good to go. Let me know when these are ready and I'll be happy to review the PR.

ElDeveloper commented 2 years ago

@ahdilmore replying to your comment:

Updated: I'm actually not sure what the best route to take here is. Would you suggest _validate_sample_sheet_metadata to check that the specific values are not None or were you thinking something else?

Got it. My recommendation is to make this an error (not having these values would be bad for the study in Qiita), and to put the validation in _validate_sample_sheet_metadata. What do you think? Maybe make the default value of experiment_design_description an empty string and error if it is None, it's not present or it's an empty string.

ElDeveloper commented 2 years ago

@ahdilmore, with the PR from @callaband just being merged I realize that there will be a few conflicts that might be hard to solve. Namely for amplicon-pooling.ipynb I suggest that you get the latest copy of the notebook from the repo and make your updates there (solving conflicts on notebooks sounds hard 😟 ). For the other files, I would suggest to re-generate them after you have updated the notebook.

ahdilmore commented 2 years ago

@ElDeveloper yep, was just running into these issues now 😅. Going to pull Celeste's changes and then work from there

ahdilmore commented 2 years ago

Should be ready now, @ElDeveloper! We may need to update the other notebooks with example values for experiment_design_description.

ElDeveloper commented 2 years ago

Thanks so much @ahdilmore! Looks great to me.