search

matsengrp / cft

Clonal family tree
5 stars 3 forks source link

Default naive name #253

Closed metasoarous closed 5 years ago

metasoarous commented 5 years ago

@psathyrella I changed the default inferred naive sequence name in CFT to inferred_naive. Hopefully this is less likely to clash (per your motivation for setting it to X-naive-X) than naive, while also being on point semantically. My inclination is towards a sane (and unbreaking) default, because people really shouldn't have any reason to set this unless they know for certain that its going to be problem. And this matters quite a bit in this instance because if someone forgot to set the --inferred-naive-name setting running scons, it would clobber all of their previous results instead of doing an incremental/partial build. So we shouldn't be expecting people to add this flag just to avoid having weird sequence names in their output.

I'm not set on inferred_naive specifically if you have another suggestion. If we want something "globally unique", we can also use namespaced keywords, which are already a thing in the metadata output (probably something like cft.seq:inferred_naive). There's a chance that : could be problematic in some software or another though, so this may not be the best solution here. Could always give it a whirl of course.

psathyrella commented 5 years ago

No, that sounds fine. I guess I would say that it's a tradeoff between likelihood-to-conflict and nice semantics, though -- x-naive-x looks ugly, but that's also why it's less likely to conflict by accident with an input name that someone chooses. That said it shouldn't be a problem, since I think we just crash with an informative error message if someone passes in a conflicting name, at which point they can either set the option or change the name of their sequence.

metasoarous commented 5 years ago

@psathyrella OK; Thanks for the feedback. Let's stick with this then.