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 attributes mito_name and macs_gsize to fasta asset #2

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

mirpedrol
Copy link

@mirpedrol mirpedrol commented Jul 15, 2022

Add a new asset class child from "fasta" asset class. Add attributes mito_name and macs_gsize.

@mirpedrol
Copy link
Author

Hi @nsheff!
I was trying to add attributes to the fasta asset as we were discussing.
I can't find documentation about this kind of assets, is there a way to test that before merging the PR?
Thanks!

@mirpedrol
Copy link
Author

Thank you very much for your review @nsheff.
These attributes are values that we provide by had (example), can I do this by creating an env variable? Or must they be obtained from a fasta file?

@nsheff
Copy link
Contributor

nsheff commented Dec 13, 2022

Got it, now I remember what you're trying to do here. I think this is the way to do this:

  1. for the seek keys, use value: "{params.name}" and value: "{params.size}" to access the input parameters.
  2. I believe your command_template_list should just be empty -- those values will automatically be made available as seek keys in the params namespace.

That should work... I'll try to test later today.

But the other idea is for this to instead extend the fasta asset, instead of being an entirely new asset type. In that case you'd want to do it a bit differently -- these seek keys would be the same, but then we'd be adding them to a typical fasta asset, so you'd do something like seek genome/fasta.name

So in other words, the question is just: should this be a separate asset that uses a fasta asset as input? Or should it be a child of the fasta asset, such that it is a fasta asset with some extra capabilities?

@mirpedrol
Copy link
Author

Ah ok I get it now, thanks for the clarification.
Both options should work for us, but I think the second one makes more sense. I will try to add the changes 🙂
The only potential problem I can think of now is the case where we only have one of these two variables, will this be a problem for the asset? I assume we can always set them as empty strings.

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.

None yet

2 participants