Add comment about setting a default prefix that isn't just meta.id
@netlify /docs/guidelines/components/modules
Deploy Preview for nf-core-main-site ready!
| Name | Link |
|---|---|
| Latest commit | db059e4628c4c14b9b917641045a694d72b66644 |
| Latest deploy log | https://app.netlify.com/sites/nf-core-main-site/deploys/67a074cc8226c8000851e454 |
| Deploy Preview | https://deploy-preview-2608--nf-core-main-site.netlify.app/docs/guidelines/components/modules |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
failing playwright tests can be ignored for now
What is the state of this docs update, @SPPearce ?
What is the state of this docs update, @SPPearce ?
@jfy133 and @nvnieuwk are still against it as far as I know, but I still don't understand why. The suggestion was to discuss at a maintainers meeting but I was on leave for the one last week.
What is the state of this docs update, @SPPearce ?
@jfy133 and @nvnieuwk are still against it as far as I know, but I still don't understand why. The suggestion was to discuss at a maintainers meeting but I was on leave for the one last week.
ah, yes, remember now. let's try to bring it up in the next meeting then.
Sorry for being difficult :grin: let's talk about it in the next meeting indeed :)
Deploy Preview for nf-core-docs ready!
| Name | Link |
|---|---|
| Latest commit | db059e4628c4c14b9b917641045a694d72b66644 |
| Latest deploy log | https://app.netlify.com/sites/nf-core-docs/deploys/67a074ccf106910008a7bcd3 |
| Deploy Preview | https://deploy-preview-2608--nf-core-docs.netlify.app/docs/guidelines/components/modules |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
#small-mound-i-feel-strongly-about-but-not-enough-to-fight-everyone
The issue is more we still don't understand the objection. Since you're experienced, at least I feel like, there is something we're not understanding about your viewpoint.
We've established though that the developer still has control.
The issue is more we still don't understand the objection. Since you're experienced, at least I feel like, there is something we're not understanding about your viewpoint.
We've established though that the developer still has control.
I can't remember the depths of the discussion anymore, but mostly it's a development style.
It firstly irks me because I firstly hate pipelines that tag loads of extra suffixes at the end (_sorted_indexed_sorted_indexed), however shouldn't happen here because of course it's just meta.id + prefix/suffix every time.
But the main reason is: I strongly feel that developers should think very carefully about how output is presented to users. Automating output names with defaults to making the development experience 'easier' at the expense user experience is not a good practice in my personal opinion. Sure, it can be overridden by an organised pipeline developer, however most people are too busy and will take any opportunity to skip steps if they can. By allowing default prefixes it'll make people not think about this, and thus not think carefully about what goes in the results directory etc and just result in big mess and harder and less attractive to use the output. This of course would be mitigated with good documentation, but that still remains poor across all of bioinformatics (/rant).
I personally prefer having a hard error when having a name conflict, as it forces the developer to think carefully about the name, then should it even be presented to the user, and then logically where should it go in the output directory etc etc. Rather than 'hoping' they'll do some TLC.
But I do recognise that this is a personal opinion/development style about a relatively minor point so I won't block it for that reason when I've been outvoted ;).
It firstly irks me because I firstly hate pipelines that tag loads of extra suffixes at the end (
_sorted_indexed_sorted_indexed), however shouldn't happen here because of course it's just meta.id + prefix/suffix every time.
Agreed. Perhaps this should be a pipeline linting check (warning) or an nf-test check. Split filenames on underscores/periods/non-alpha-numeric characters and check the number of unique parts against total (duplicate words) and that the total is not more than say 7 (ultra-long names).
But the main reason is: I strongly feel that developers should think very carefully about how output is presented to users. Automating output names with defaults to making the development experience 'easier' at the expense user experience is not a good practice in my personal opinion. Sure, it can be overridden by an organised pipeline developer, however most people are too busy and will take any opportunity to skip steps if they can. By allowing default prefixes it'll make people not think about this, and thus not think carefully about what goes in the results directory etc and just result in big mess and harder and less attractive to use the output. This of course would be mitigated with good documentation, but that still remains poor across all of bioinformatics (/rant).
I think we all agree on the first sentence. I disagree that this change is at expense of user experience though. While it makes developer experience better by not automatically resulting in a filename collision, the user should see at most one _something tacked on ( maybe we need to be explicit about how the default prefix can differ from meta.id, for example the default prefix cannot be based on file.baseName ). So then even in a chain of these modules with defaults changed from meta.id because they might commonly result in collision, there should be at most one _something.
Conversely though, are we currently making the user experience better by how we're setting the meta.id? For many developers it's just a key to make something unique for joining. A busy developer is just as likely to skip proper value setting of meta.id too.
I personally prefer having a hard error when having a name conflict, as it forces the developer to think carefully about the name, then should it even be presented to the user, and then logically where should it go in the output directory etc etc. Rather than 'hoping' they'll do some TLC.
I think this is the key part though. Putting in developer roadblocks means the developer has to directly act. I don't think it'll force them to put TLC into it though. Workflow design ( including naming outputs, etc) is still a skill imo.
Sorry, I guess this wall of text wasn't necessary, but maybe there is one thing we still need to do, and that's define how much the prefix can differ from meta.id.
Huh, I thought this had actually been merged a while ago. Can we merge this now?