Incorporate matrix tube sample accession into amplicon notebook v2 #200
Conversation
charles-cowart
requested review from
antgonza,
RodolfoSalido,
mmbryant23 and
cbrenchy
charles-cowart
changed the title
|
Ostensibly ready for review. It is still a big one! However, some of the changes are just new files and changes that can be reviewed quickly. Two version 2 scripts were added, since I couldn't audit their behavior on my own. @cbrenchy and I will review those once the merge has occurred. |
…control name generation. Also the iseq_count notebook for generating katharoseq names
| @@ -2,8 +2,10 @@ | |||
| "zymo_dna_rna_shield": | |||
| "storage_liquid_lot_number": | |||
| "219839": | |||
| "mass_storage_tube_and_storage_liquid_before_sample_g": 16.9 | |||
There was a problem hiding this comment.
storage tube masses should not be found in this .yml file anymore
There was a problem hiding this comment.
... just to confirm, where are they coming from? Any other columns not needed any longer?
There was a problem hiding this comment.
Going forward, the Mayo is going to weigh every tube individually and provide that value in a "mass_storage_tube_and_storage_liquid_before_sample_g" column in the metadata they send us (instead of us adding that column and filling it in based on the lot average).
There was a problem hiding this comment.
Thank you. Just to confirm, should we assume that this will be part of the sample or the prep-info file? and what should be the role of the SPP here?
There was a problem hiding this comment.
Thank you for the catch @mmbryant23 ! @antgonza they were most likely recently removed and I merged them back in by accident from Caitriona's branch. Things recently removed from the master can look like new lines of code/text in the feature branch. I'll remove them.
|
Thanks, Charlie!
Antonio - Tube weights will be a part of the sample information file, but
this .yml file does not play a role in the SPP. It is utilized in the NPH
calculation notebook.
…On Tue, Apr 30, 2024 at 12:33 PM Antonio Gonzalez ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In metapool/data/abs_quant_sample_info_calc.yml
<#200 (comment)>
:
> @@ -2,8 +2,10 @@
"zymo_dna_rna_shield":
"storage_liquid_lot_number":
"219839":
+ "mass_storage_tube_and_storage_liquid_before_sample_g": 16.9
Thank you. Just to confirm, should we assume that this will be part of the
sample or the prep-info file? and what should be the role of the SPP here?
—
Reply to this email directly, view it on GitHub
<#200 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AVY3ITXGMZX6OIDOSNT2ZILY77BWHAVCNFSM6AAAAABG3XMSYOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMZRHE3DGNRSGE>
.
You are receiving this because your review was requested.Message ID:
***@***.***
com>
|
|
A quick update: The PR is passing all of its tests and lints correctly w/flake8. It looks like there's an issue with code-coverage calculation and coveralls. That doesn't appear to be related to the codebase, since none of that has recently changed. I'll give it some time and rerun the coverage generation later. |
|
@antgonza ready for review! The appearance of the error in coverage didn't appear to be caused by a version change; I confirmed the versions did not change from the last successful version until now. There does appear to be an issue for some people who experience this error, possibly because a .pyc file was introduced at one point into the repository: https://stackoverflow.com/questions/2386975/no-source-for-code-message-in-coverage-py Ultimately I created a .coveragerc file which tells AndreMiras/coveralls-python-action@develop to ignore all errors when finishing coveralls. This resolves the issue and appears to be all right - it doesn't hide errors related to unit-tests for instance. |
From @cbrenchy: To incorporate matrix tube sample accession, randomization and control naming schemes into the amplicon notebook. Specific updates: Update Test Data, change append function to pd.concat in optional part of amplicon notebook, add additional columns to prep file and update control names.
From @RodolfoSalido: Addresses #169
From @charles-cowart: Merges all changes w/latest codebase. All affected tests were reviewed and updated. All tests pass. Some additional error-handling added.