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.

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

Add dsb component #588

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Add dsb component #588

wants to merge 5 commits into from

Conversation

wxicu
Copy link
Collaborator

@wxicu wxicu commented Oct 17, 2023

Changelog

Add dsb component: which splits the dsb function into two components, the first takes care of recognising the barcodes for foreground and the background, while the second does the actual normalization

Checklist before requesting a review

  • I have performed a self-review of my code

  • Conforms to the Contributor's guide

  • Check the correct box. Does this PR contain:

    • Breaking changes
    • New functionality
    • Major changes
    • Minor changes
    • Documentation
    • Bug fixes
  • Proposed changes are described in the CHANGELOG.md

  • CI tests succeed!

Copy link
Contributor

@rcannood rcannood left a comment

Choose a reason for hiding this comment

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

Hi @wxicu !

See my comments provided separately.

I'd suggest replacing --data_raw with --input and make it a MuData file. What do you think?

namespace: protein_processing
description: "Filter background and foreground signals for normalising protein expression with DSB (Denoised and Scaled by Background)."
authors:
- name: Xichen Wu
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the author yaml to insert author information

Suggested change
- name: Xichen Wu
- __merge__: /src/authors/xichen_wu.yaml
roles: [ maintainer, author ]

namespace: protein_processing
description: "Normalize protein expression with DSB (Denoised and Scaled by Background)."
authors:
- name: Xichen Wu
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the author yaml to insert author information

Suggested change
- name: Xichen Wu
- __merge__: /src/authors/xichen_wu.yaml
roles: [ maintainer, author ]

Comment on lines +8 to +11
- name: "--data_raw"
type: file
required: true
description: "A ``MuData`` object containing raw (unfiltered, including empty droplets) data for both ``prot`` and ``rna`` modalities."
Copy link
Contributor

Choose a reason for hiding this comment

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

We typically use --input to denote an input mudata object in openpipelines

Suggested change
- name: "--data_raw"
type: file
required: true
description: "A ``MuData`` object containing raw (unfiltered, including empty droplets) data for both ``prot`` and ``rna`` modalities."
- name: "--input"
type: file
required: true
description: "A ``MuData`` object containing raw (unfiltered, including empty droplets) data for both ``prot`` and ``rna`` modalities."
example: input.h5mu

type: file
direction: output
description: dsb_index output directory
example: "dsb_output"
Copy link
Contributor

Choose a reason for hiding this comment

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

Make this a required argument and add a slash to the example to denote it's a directory

Suggested change
example: "dsb_output"
example: "dsb_output/"
required: true

multiple: true
- name: "--output"
type: file
direction: output
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be inclined to make the output an h5mu file instead of a directory, because then it plays much nicer with the other components in OpenPipeline. If you want, we can discuss this in the next OpenPipelines working meeting.

@rcannood
Copy link
Contributor

Hi @wxicu ! Just following up on this PR. Do you have any thoughts about the comments I placed?

@wxicu
Copy link
Collaborator Author

wxicu commented Feb 19, 2024

Hi @wxicu ! Just following up on this PR. Do you have any thoughts about the comments I placed?

Hey really sorry for my late response. Finally have time now to contribute on this. I will go through all the comments and update it soon! Again, really sorry for the delay

@DriesSchaumont DriesSchaumont marked this pull request as draft August 12, 2024 11:25
@rcannood rcannood self-assigned this Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants