https://nf-co.re logo
Join Slack
Powered by
thread3: strandeness
# team-maintainers
m
thread3: strandeness
should we or not put strandeness as an official meta similar to single_end? for me, single_end belongs in an official meta as it changes the way we invoke tools strandeness could be handled via args, like we do with any other condition, but it is widely use, so there is definitively a case for this pro: it will simplify our lives con: it can be done via args
Ok, so in my personal opinion, if I were to make an executive decision, I would against my own opinion allow it, just for the sake of making our life easy. @James Fellows Yates you are the voice of reason
j
Honestly guys, really, I just want a peaceful life. I could use strandedness before, now I can't because someone disallowed it and I'm having to deal with the consequent fallout in #CE8SSJV3N. If you're going to re-allow it could you do it quick, yeah? 😛 (disclaimer: I have man flu, patience level may be low!)
🤣 2
j
Both single_end and strandedness should be process inputs if their usage is mandatory or affects inputs/outputs. Otherwise they should be args. (and args should be mandatory inputs! 🥹😜)
j
I was literally about to merge this: github.com/nf-core/modules/pull/12979 ... removing the use of meta.strandedness in the process. So I'd appreciate a call being made quickly 😄
m
Given the discussion, I am making the decision. Let's have meta.strandeness as an official meta field. Force merge for now, we will update documentation and tools.
do it 1
j
OK cool! Expect pestering for force-merges
@Maxime Garcia care to cast your eyes over this one then? github.com/nf-core/modules/pull/12979. It's already approved, and does a lot besides the strandedness thing (the Salmon module seemed to have been behind standards in a few ways). Lint now fails of course, but maybe you could use your big stick if you're happy?
👀 1
m
Really minor comments Only issue I can argue is that we could use sanitizeOutput on the tests for nicer and easier snapshots
j
OK, will mop up
s
Can you set your agent onto making tools allow it? 😉
j
I can do it with caveman hands
s
you'll need to update the tests too 😉
(especially as the tests include a test to make meta.strandedness explicitly not allowed)
j
Damn my caveman hands
s
I feel this is slightly like a coup 😉
🤣 1
m
Revolution
j
Should we have wider discussion? If anyone says RFC the kitten gets it
j
can you make an rfc about making an rfc please
screams 2
j
OK, in fairness, folks should opine if they really hate the idea of a blessed strandedness property: github.com/nf-core/tools/pull/4473
a
Do you just want it as a comment on the PR? I'm against this because I don't like these weird exception properties which are not relevant to 90% of users. I've never had to use single_end or strandedness in the situations we have allowed exemptions for them - indeed I've even had to spoof/inject the single_end into a meta for which it was meaningless just to make a module work.
j
indeed I've even had to spoof/inject the single_end into a meta for which it was meaningless just to make a module work.
I have also have had to do this before. I agree we should ditch them, but I'm not opposed to grandfathering them in for older modules, as long as we remove them as we update them. Presumably this particular problem is sorted by records in some way anyway.
a
Not really solved by records. This is about whether we allow hardcoded references to meta values within the module code, records or not it's the same concept I think?
j
Depends on how we implement records. If every module takes in a single record, one of the fields will have to be e.g. 
single_end
if it is needed.
a
That's true if we are saying eg single_end should move from meta to a first-class input, but false if we are saying it should move from used from meta hardcoded in module logic to used from meta in args logic
j
I think that's the crux of it. 
single_end
should be a mandatory input if affects inputs or outputs, and an arg if it doesn't. Same with 
strandedness
this 3
m
I would tend to agree, but that's too much changes for such a quick decision, I cannot force that one
j
Let's not get into that one until we start experimenting with records, which I need to do soon
👍 1
j
I think we can grandfather in 
single_end
and 
strandedness
and say that they are allowed, while working to migrate modules where we can
j
OK fine. Damn you democracy shakefist
I'll toe the linter line for now
n
Sounds like a fun hackathon project to migrate everything to inputs/args ;)
j
shared a GIF
🤣 3
n
(Sorry couldn't stop myself 😁)
😆 2
m
SORRY, I am now realising I was connected as the github bot
🤖 1
facepalm
@Jonathan Manning I have changed the based branch of your PR and solved merged conflicts https://github.com/nf-core/tools/pull/4473
j
I've left it as draft, I don't think we should actually stage a coup, even as disillusioned as I am with democracy
m
I do not think it's a coup, I will speak with the rest of core team and defend it, it's a #C0749PAEU9F, so much is breaking because of that decision, and I do think this would make our lives easier, while we wait for a magic solution
the #C0C3NMTFWN4 is a success, we have an approval from the big man upstairs himself. I'll whip out a PR to update website
j
shared a GIF
m
j
Is it worth saying something to the effect that their use is discouraged in new modules?
a
I would argue go further than discouraged and say they "SHOULD/MUST NOT" be used in a new module with the exception of a sibling module of a took using the pattern already?
party this 2
m
updated the PR
Thanks @Daniel Lundin
2 Views
Open in Slack
PreviousNext
Powered by