-
Notifications
You must be signed in to change notification settings - Fork 10
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
Tweak print method to show altExpNames #85
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.
Thanks! Well done.
For some reason, you have conflicts to solve (see above). Maybe recent PR changes have the same lines you change. Please have a look and solve the conflicts.
Also, I see you have two branches, but you are doing the PR just for master
. Not sure if this is intended.
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.
Also, please add a unit test, add a dummy "alternative experiment", and verify that the print looks as expected.
paste(assay_names, collapse=", ") | ||
), after=1) | ||
append(sprintf( | ||
"\033[90m Features=%s | Cells=%s | Assays=%s | altExpNames=%s\033[39m", |
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.
Thanks.
I bit more elegant would be
- Replace
altExpNames
withAssays_alternative
- Conditionally add "| altExpNames=%s" if alternative experiment exists
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.
Fair point. I had considered adding the information on a conditional basis but personally I prefer to know even if it's empty. The standard SCE
print method outputs altExpNames(0):
when empty and altExpNames(1): Antibody Capture
when it is populated (Antibody Capture
being the name of the alternative experiment and can be any string). If you feel you would rather keep it only when it is non-NULL I can do that quite easily.
In relation to the naming, it actually refers to the name of the experiment rather than the names of the assays, i.e. if it exists you will still have counts
and normcounts
or other as the assay names within the altExp
slot. How about saying Other_experiments=
?
I will also add the unit tests and other missing bits whilst I am at it.
Thanks.
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.
I see.
let's try to generalise the concept of experiment and assay, still hinting at the non-modular nature of singlecellexperiment
.
So I am in favour of printing the experiments within the object, if they are 1,2,3, independently whether they are base or alternative.
having said that, there are a lot of ways to go about that. One possibility is to do
Assays= base:counts,logcounts; Antibody capture:counts,logcounts; ADT:counts
Other ideas are welcome!
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.
@Biomiha, I realised that the sentence "OK, the goal here is to provide an interface that promotes modularity. " sounded very bad in writing; the tone I was thinking was completely different from how you would read it!
I used a poor form to give the broad indication that "the tidy transcriptomics thrives to giving modularity (which sometimes diverges from the Bioc framework)."
I hope you did not take it personally!
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.
No dude, absolutely not. I work in biotech 😄 .
I agree with what you've said. Am working on implementation, just been very busy lately.
Add printing of altExpNames if they exist or NULL if there is nothing.