-
Notifications
You must be signed in to change notification settings - Fork 4
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
fix: make variable naming consistent with ISA standard #13
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I agree with harmonising this overal, I'm not sure that a blanket change to all variable names is the right approach here:
- for one thing, the stem cell core templates actually ask for sample names, since we use the cellline names as source
- From what I've seen so far many templates use the same name for sample & source. While this is of course not ideal/desribale, it is often not quite avoidable without a specific experimental design (that the cookiecutter templates can't really build upon since the have to be very general). Therefore the cookiecutter json may conceptually only ask for a single 'name' for a sample and then use that for both source and sample name. For most experimental people (or just people not accustomed to ISA) sample name is much more intuitive description than source name in these cases.
...es/isatab-stem_cell_core_sc/{{cookiecutter.__output_dir}}/s_{{cookiecutter.s_file_name}}.txt
Outdated
Show resolved
Hide resolved
...es/isatab-stem_cell_core_sc/{{cookiecutter.__output_dir}}/s_{{cookiecutter.s_file_name}}.txt
Outdated
Show resolved
Hide resolved
...{cookiecutter.__output_dir}}/a_{{cookiecutter.assay_prefix}}_{{cookiecutter.assay_name}}.txt
Outdated
Show resolved
Hide resolved
Another thing (I almost forgot): This only seems to the address the variable names, however many templates also use |
Thanks Nicolai for looking into this.
That was indeed an oversight in the stem_cell_core_sc template which is fixed now. I left _bulk unchanged because of this.
Most templates derive their Source Names from the Sample Names, but I would agree with Mikko that this is a bit confusing in the context of ISA-tabs and also experimentally. I would expect Sample Names to be derived from the Source Names plus a suffix (optionally). That is how I defined it in for the MC template, there is
Not sure what you mean by this. s_ files need to start with a Source Name column to be standard compliant and all do so right now. |
The templates might do indeed do this, but I would argue that most users generally do not, since they only come up with source names when they start entering things into sodar (they will always have some sort of sample name ready).
Ah you're right I must have confused some (older?) things here or maybe I just remebered the start of some a-files ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code wise the changes all look good to me.
I have some concerns about the idea/purpose of this change (see my other comment), but that's not easily addressed either way.
As this is not really urgent, let's not do anything hastily and talk once I'm back in Berlin. |
fixes: bihealth/cubi-tk#106