Workshop feedback

[multimeric]

Just a list of thoughts I had from running the workshop last week. Overall the workshop was excellent and very clear, so don't think this is any serious criticism, it's just hard to give feedback on things that went well!

Oh, and also these are very low priority, the workshop doesn't need any of these changed urgently or at all, I'm just writing everything down while I remember.

• I forgot to update the list of required packages with the move to crew (whoops!): https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/learners/setup.md#L15-L27
• Why do we tell the users to install crew at this point (previously batchtools) if we do so in setup.md? https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/parallel.Rmd#L61-L63
• Wasn't sure what this meant: https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/introduction.Rmd#L47
• It was a touch confusing to go from looking at _targets.R to jumping backwards into non-targets code: https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/basic-targets.Rmd#L119-L122. However I'm not sure how to improve this.
• The non-targets code can be all run in a terminal, without using files, although this could be made clearer: https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/basic-targets.Rmd#L130-L138
• This is probably a controversial take but tar_load and tar_read are so similar in function, it hardly seems worth teaching both: https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/cache.Rmd#L97
• I wasn't so sure about _targets/user. Although it's mentioned in the docs somewhere, I don't fully understand why we would put files there instead of just in our project root. I haven't personally ever used this directory: https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/cache.Rmd#L140-L142
• An example way of modifying the workflow without breaking it would have helped here. I just added an empty line with just 1 on it, but that was just off the top of my head: https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/lifecycle.Rmd#L214
• Ideally I think we could have illustrated this point using the workflow we just ran. For example could we modify the file path function but make it return the same string to prove that the subsequent tasks don't have to re-run? https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/lifecycle.Rmd#L229
• I found tar_outdated and tar_progress a bit superflous since tar_visnetwork and the regular tar_make reporting mostly cover this info: https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/lifecycle.Rmd#L245
• An example output from tar_make(some_target) would be useful. I did this when I ran the workshop but it wasn't in the material: https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/lifecycle.Rmd#L300-L301
• I didn't find tar_plan so much simpler that it was worth confusing learners with two options: https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/organization.Rmd#L37
• Another Hot Take, but I wonder if we needed to teach so many methods of loading packages? I like the idea of not teaching library, just because it's almost strictly worse than tar_options(packages=). The other methods have reasonable tradeoffs that I can see the value in teaching though: https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/packages.Rmd#L41
• The conflicted section didn't seem entirely necessary, because although it's a useful package to know about, it's not an inherent problem with Targets: https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/packages.Rmd#L229
• tar_file_read didn't (doesn't) seem massively important to me, because it's basically as concise if we just write the two targets ourselves. Plus I imagine the !!x syntax is pretty confusing to the uninitiated: https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/files.Rmd#L154
• I was asked a question about why we do models[[1]], which lead to an interesting diversion into the iteration argument to tar_target(). I wonder if this would be worth talking about specifically. On the other hand, I understand why you didn't use iteration="list" here, because you wanted to retain the names: https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/branch.Rmd#L294
• Not sure why R universe is needed here? https://github.com/carpentries-incubator/targets-workshop/blob/437601110657240a276c0823ed9f7096cb209895/episodes/parallel.Rmd#L68-L70

[joelnitta]

Thanks so much for this great feedback! It will take me some time, but I will try to address these. Some may be split off into their own issues.

Many of these come back to a common problem - how much information is the right amount to present during the workshop. It is tempting to try and be comprehensive, but that can make learning harder if there is too much information and not all of it is necessary to grasp the concepts.

[multimeric]

Hi @joelnitta, did you get a chance to think about any of these? I'm still in favour of removing tar_plan and tar_file for example, and I'm happy to submit a PR.

[joelnitta]

Thanks again for the feedback!

Here is a summary of my responses. I've split out most of these into separate issues.

• [x] I forgot to update the list of required packages with the move to crew

  • Fixed by #32

• [x] Why do we tell the users to install crew at this point (previously batchtools) if we do so in setup.md?

  • Fixed by #32

• [ ] Wasn't sure what this meant

  • I was thinking of branching - you can give targets a pattern, and it will define the workflow steps (targets) for you.

• [ ] It was a touch confusing to go from looking at _targets.R to jumping backwards into non-targets code

  • Opened #33 for this.

• [ ] The non-targets code can be all run in a terminal, without using files, although this could be made clearer

  • Not quite sure what you mean by "all run in a terminal"

• [ ] This is probably a controversial take but tar_load and tar_read are so similar in function, it hardly seems worth teaching both

  • Opened #34 for this

• [ ] I wasn't so sure about _targets/user. Although it's mentioned in the docs somewhere, I don't fully understand why we would put files there instead of just in our project root. I haven't personally ever used this directory

  • Opened #35 for this

• [ ] An example way of modifying the workflow without breaking it would have helped here. I just added an empty line with just 1 on it, but that was just off the top of my head

  • Opened #36 for this

• [ ] Ideally I think we could have illustrated this point using the workflow we just ran. For example could we modify the file path function but make it return the same string to prove that the subsequent tasks don't have to re-run?

  • Opened #37 for this

• [ ] I found tar_outdated and tar_progress a bit superflous since tar_visnetwork and the regular tar_make reporting mostly cover this info

  • Opened #38 for this

• [ ] An example output from tar_make(some_target) would be useful. I did this when I ran the workshop but it wasn't in the material

  • Opened #39 for this

• [ ] I didn't find tar_plan so much simpler that it was worth confusing learners with two options

  • Opened #40 for this

• [ ] Another Hot Take, but I wonder if we needed to teach so many methods of loading packages? I like the idea of not teaching library, just because it's almost strictly worse than tar_options(packages=). The other methods have reasonable tradeoffs that I can see the value in teaching though

  • If we chose one, I'd lean toward library() actually, for two reasons: 1) it's what people know already, 2) it is easier to use with renv (beyond the scope of this lesson, but a logical next step on learners' reproducibility journeys)

• [ ] The conflicted section didn't seem entirely necessary, because although it's a useful package to know about, it's not an inherent problem with Targets

  • Opened #41 for this

• [ ] tar_file_read didn't (doesn't) seem massively important to me, because it's basically as concise if we just write the two targets ourselves. Plus I imagine the !!x syntax is pretty confusing to the uninitiated

  • Opened #42 for this

• [ ] I was asked a question about why we do models[[1]], which lead to an interesting diversion into the iteration argument to tar_target(). I wonder if this would be worth talking about specifically. On the other hand, I understand why you didn't use iteration="list" here, because you wanted to retain the names

  • Opened #43 for this

• [x] Not sure why R universe is needed here?

  • Fixed by #32

[multimeric]

I just ran this workshop again. I want to re-iterate all the previous points I made about cutting out some of the alternatives we discuss, like all the package loading strategies, as I ran out of time.

The named list strategy (combined_model = lm(bill_depth_mm ~ bill_length_mm, data = penguins_data)) caused confusion again. Could we maybe use a data frame here, because then I think we can get "names" as well as avoiding the iteration argument.

One new point was that I don't have a good explanation for why tar_file works for both input and output files. This isn't really explained in the material either.

[joelnitta]

@multimeric Thanks!

I am (finally!) going through your comments more carefully as you can see above and opening issues as needed. I will reply to your comments about named list and tar_file as I get to them.

I am also teaching this again soon, so I really appreciate the additional feedback 🙏