DOI-USGS / ds-pipelines-targets-3-course

Many-task pipelines using targets
Other
4 stars 6 forks source link

Noteable issues/changes during walkthroughs #10

Closed lindsayplatt closed 3 years ago

lindsayplatt commented 3 years ago

As folks take the course, please log your stuff here :) I will plan to address before the big group of new hires is onboarded!

hcorson-dosch-usgs commented 3 years ago

Tiny typo in Issue 6 - Splitters:

Add a code to subset the rows in oldest_active_sites based on the branching variable, state_abb. Remember that oldest_active_sites has a column called state_cd containing the state abbreviations. Hint: go peek at the first line of the function get_site_data() in 1_fetch/src/get_site_data.R.

"Add a code" should be "Add code"

hcorson-dosch-usgs commented 3 years ago

In the 4th comment on the Appliers Issue (number 8) , the bot says:

To help you assess your pipeline, here's what I would have put in that comment:

And then pastes:

but does not paste in this:

hcorson-dosch-usgs commented 3 years ago

In the Scale Up issue, after setting up and building the temperature pipeline, the bot prompts the taker to provide the following in a comment:

When everything has run successfully, use a comment to share the images from timeseries_KY.png, timeseries_VT.png, and data_coverage.png. Include any observations you want to share about the build.

But we can't provide a timeseries of temperature for Vermont b/c we'd just been asked to remove VT from the temperature pipeline earlier in the activity:

Remove 'VT' and 'GU' from the states target in _targets.R. It turns out that NWIS returns errors for these two states/territories for temperature, so we'll just skip them.

Edited to add

Now I'm thinking that might have been intentional? Given this next comment from the bot:

You might have been disappointed to note that there's still a timeseries_VT.png hanging out in the repository even though VT is now excluded from the inventory and the summary maps. Worse still, that file shows discharge data!

lindsayplatt commented 3 years ago

^ Yes, intentional but maybe not the best way to introduce that idea? Could just pick a different state and still have the next point be about how VT is still there, or could prompt by saying ...share the images from timeseries_KY.png and data_coverage.png. Peruse the other filenames available in that folder and then include any observations ...

lindsayplatt commented 3 years ago

To fix the missing visnetwork image, I am going to use @hcorson-dosch's image so that I don't have to get my pipeline back to this state to regenerate :)

image