https://nf-co.re logo
Join Slack
Powered by
I've been working on this PR this week. It's my fi...
# airrflow
j
I've been working on this PR this week. It's my first contribution to a nf-core pipeline. I'm available until tomorrow and then I'll be back on September 9th, I hope that won't be a problem for you. Thanks in advance for the review. https://github.com/nf-core/airrflow/pull/343
g
Thanks @Justine, I’ll have a look until you return 🙂
j
Hi @Gisela Gabernet, why remove specific_5p_race_umi from nextflow_schema.json ? I can't find any comments on my pull request, what should I do next please ? Thank you very much.
Hi @Gisela Gabernet and @ssnn, can you give me some feedback on my PR https://github.com/nf-core/airrflow/pull/343 so that I can confirm to the research teams at the research unit I work that my additions will be included in the next pipeline release? Do you know when the new release will be in preparation ? That'll allow me to plan a period to finalize the PR if there are things to do. I've also been working on adding the amino acid physico-chemical properties of alakazam to the enchantr package, so knowing if there's also an enchantr release planned would allow me to plan the right period to work on it. Thank you very much. Best regards, Justine
g
Hi Justine, apologies as I see now that I had started a review but never made it public. I'm reviewing the code now. Could you provide a bit more detail on the library design for which this new protocol 
specific_5p_race_umi
would apply? Maybe a quick diagram would help
I was planning on releasing soon: this or next week as I want to add a small feature to it and have the current features out.
j
I understand, thank you for your answer. I'll see about getting this diagram and the difference with the dt_5p_race_umi. The airrflow release is scheduled for the next 2 weeks, right?
Hi @Gisela Gabernet, My colleague came to tell me that she was going to draw the diagram. It's an homemade oligo carrying the UMI + a sequence called CAP (HomeMadeOligo-UMI-CAP) which will hybridize in 5' of V(D)J region of the sequence resulting from reverse transcription thanks to this CAP sequence. As reverse transcription was initiated by the C region primer in 3' of the V(D)J region, the R2 start directly with the C region. The R1 start with the HomeMadeOligo-UMI-CAP so in my fasta that I give via the race_linker option I give the UMI-CAP sequence in order to cut the HomeMadeOligo sequence. I don't know if I'm explaining myself properly. If I've understood correctly what I still have to do, I need to set aside some time this week to write the test that corresponds to the test-dataset that I'm going to add to a new folder in the airrflow branch of nf-core/test-datasets github repo, right ? Sorry, It's difficult for me to understand the results of the review. Thanks in advance for your help.
g
Hi Justine, yes I think the main point to clarify is to know whether this analysis needs a new protocol or it can be analyzed with the 
dt_5p_race_umi
protocol that already exists, as this would be preferable and it sounds quite similar to this one. The diagram will be really helpful!
j
Could you look here : https://github.com/nf-core/airrflow/issues/342 I've explained why it doesn't work with dt_5p_race_umi.
@Gisela Gabernet Here's a link to the exchange we had about using fastp for my inconclusive tests with dt_5p_race_umi : https://nfcore.slack.com/archives/CEJLWPB3N/p1721905015859459?thread_ts=1721905015.859459&cid=CEJLWPB3N
g
Hi @Justine, again really sorry for the late reply but things are super busy at work at the moment. I see, so the difference from the 5p_race protocol is really just cutting the sequence upstream from the UMI + race linker. I would not create a new protocol for that but just add the possibility of cutting a preceding sequence for all protocols, that will make your feature addition more useful for everyone. This is only relevant, I think, for the read that contains the UMI, as for the other read one can use Maskprimers trim anyways and remove any preceding sequence to the primer.
I’m working on writing a possible solution on your PR now
Also have you tried the 
assemblepairs_sequential
option instead of 
assemblepairs_join
? This one should do an alignment to the reference before joining the two reads so that it introduces gaps as Ns at the junction where the reads are not aligning. Otherwise I think that the reads will get anyways eliminated at the later step where they are aligned to the reference, so I’m not sure that 
assemblepairs join
helps recovering more sequences here.
To test it out, just provide the 
--assemblepairs_sequential
param.
I have worked on the changes to the sequence trimming part, but I cannot push to your branch. Would it be possible that you give me rights to push there?
There were quite some changes and it was more comfortable to work on an editor, so I’ve made a PR to your branch instead with the suggestions: https://github.com/JustBioinfo/airrflow/pull/1/files
The new parameters then can be documented with the 
nf-core schema build
command, and it would be nice to provide an example test dataset to test this new parameter
j
Hi, Thank you very much, it's a very good idea to allow everyone to trim. As for assemblepairs_sequential, it's a solution I'd already discussed with the team and I don't remember exactly why, but they didn't want to use the reference, I can ask them again if necessary. It turns out that assemblepairs_join really inflates our numbers of productive sequences at the end of the pipeline, so this option is necessary to recover more than 40% of sequences that wouldn't merge without it. Sorry about the diagram, I haven't heard from my clinical colleague who was supposed to make it. For the test sample I can provide the R1 R2 fastq.gz of 45Mo each which obtains 2306 productive sequence (with assemblepairs_join ), would that be suitable? It's not yet submitted to a database, so should I put it in a new test-dataset folder?
@Gisela Gabernet Here's my PR with the related test-dataset : https://github.com/nf-core/test-datasets/pull/1401 fastq have been reduced with 2000 random paired reads with seqtk tools.
@Gisela Gabernet I'll look into documenting the parameters but I'd like assemble_join to be taken into account otherwise it won't be added to the new parameters. I thought I had, but I think it must have been deleted since then. How can I keep assemble_join on your PR I'm a bit lost I've never done a review? I hope that my datasets will fit and that we can leave assemblepairs_join which is really essential for us as explained in my previous message. Have a good week end.
g
@Justine have you tried whether 
assemblepairs sequential
also recovers the failed reads in your data? I do not think that it is correct to use 
Assemblepairs join
for the reason that I mentioned that it will just concatenate the reads and can introduce artificial insertions and deletions, so I am not fond of supporting this option within airrflow. 
Assemblepairs sequential
is already implemented as part of the pipeline, you can test it with the param 
--assemblepairs_sequential
@Justine, regarding the physico-chemical properties addition to the enchantr package, feel free to open a PR to the new GitHub repository as well: github.com/immcantation/enchantr. I think this would fit well into the 
defineClones
report: https://github.com/immcantation/enchantr/blob/master/inst/rstudio/templates/project/define_clones_project_files/index.Rmd. We do enchantR releases often so we could release again once this is in 🙂
The airrflow 4.2.0 release is going to come out today or tomorrow I think, but I'm happy to do another minor release (4.2.1) as soon as the features to analyze your protocol are ready.
Open in Slack
PreviousNext
Powered by