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

Parse kallisto non-standard (spliced and unspliced) outputs #133

Merged
merged 3 commits into from Aug 18, 2022
Merged

Parse kallisto non-standard (spliced and unspliced) outputs #133

merged 3 commits into from Aug 18, 2022

Conversation

fmalmeida
Copy link
Contributor

This is related to issue #116 that was being problematic for a few reasons such as:

  • Asking the same amount of memory that was being set as limit
  • Not taking into account the different outputs produced by kallisto standard and non-standard workflows.

@fmalmeida fmalmeida requested a review from grst July 21, 2022 12:41
@fmalmeida fmalmeida changed the base branch from master to dev July 21, 2022 12:42
@nf-core nf-core deleted a comment from github-actions bot Jul 21, 2022
@github-actions
Copy link

github-actions bot commented Jul 21, 2022 •
edited

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit ec5b7a4

+| ✅ 148 tests passed       |+
!| ❗   1 tests had warnings |!

❗ Test warnings:

  • readme - README did not have a Nextflow minimum version badge.

✅ Tests passed:

Run details

  • nf-core/tools version 2.4.1
  • Run at 2022-08-15 10:14:35

@grst
Copy link
Member

grst commented Jul 25, 2022

The handling of the spliced/unspliced matrices looks fine! Can we add a non-standard kallisto workflow to the CI tests?

Why was the conversion_to_h5ad subworkflow deleted?

@fmalmeida
Copy link
Contributor Author

I deleted because it was just a backup of a previous version.

If you check, its modules are still available inside the mtx_conversion.nf files.

I don’t think we can add then on testes because they require more memory 🥲

@grst
Copy link
Member

grst commented Jul 25, 2022

I see, fair enough!

@fmalmeida
Copy link
Contributor Author

Waiting for the test to be executed with dataset that was used when observing this issue:

@fmalmeida fmalmeida marked this pull request as ready for review August 15, 2022 08:22
@nf-core nf-core deleted a comment from github-actions bot Aug 15, 2022
@fmalmeida
Copy link
Contributor Author

Hi @grst and @Khajidu,
How are your thoughts on this one?
Should we consider it done? Or there is still something that should be addressed on this PR?
I ask because we also had not got much updates on its tests on slack.
😄

@grst
Copy link
Member

grst commented Aug 18, 2022

LGTM!

At some point it would be nice to get the spliced and unspliced matrices into a single anndata object (for downstream analyses with e.g. scvelo). But I would leave that for now and tackle this when we add rna velocity support also for the other aligners (tracked in #14).

@fmalmeida fmalmeida merged commit 5e51972 into dev Aug 18, 2022
@fmalmeida fmalmeida deleted the kallisto-fix branch August 18, 2022 11:23
@fmalmeida
Copy link
Contributor Author

fmalmeida commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grst grst grst approved these changes

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

Successfully merging this pull request may close these issues.

None yet
2 participants
@fmalmeida @grst