Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove plyr episode and update episode numbering #885

Merged
merged 4 commits into from
Apr 12, 2024
Merged

Remove plyr episode and update episode numbering #885

merged 4 commits into from
Apr 12, 2024

Conversation

milanmlft
Copy link
Contributor

Fixes #540

Removes the plyr episode as it has become obsolete and plyr has been retired.
I also updated the version of igraph in the package cache, as the older version was giving installation errors when trying to build the lesson locally.

Copy link

github-actions bot commented Feb 15, 2024

Thank you!

Thank you for your pull request 😃

🤖 This automated message can help you check the rendered files in your submission for clarity. If you have any questions, please feel free to open an issue in {sandpaper}.

If you have files that automatically render output (e.g. R Markdown), then you should check for the following:

  • 🎯 correct output
  • 🖼️ correct figures
  • ❓ new warnings
  • ‼️ new errors

Rendered Changes

🔍 Inspect the changes: https://github.com/swcarpentry/r-novice-gapminder/compare/md-outputs..md-outputs-PR-885

The following changes were observed in the rendered markdown documents:

 01-rstudio-intro.md                                |    2 +-
 02-project-intro.md                                |    2 +-
 03-seeking-help.md                                 |    2 +-
 13-dplyr.md => 12-dplyr.md                         |    6 +-
 12-plyr.md (gone)                                  |  509 ----------
 14-tidyr.md => 13-tidyr.md                         |    0
 15-knitr-markdown.md => 14-knitr-markdown.md       |    2 +-
 16-wrap-up.md => 15-wrap-up.md                     |    0
 config.yaml (gone)                                 |   96 --
 ...ng => 12-dplyr-rendered-unnamed-chunk-27-1.png} |  Bin
 ...ng => 12-dplyr-rendered-unnamed-chunk-28-1.png} |  Bin
 ...ng => 12-dplyr-rendered-unnamed-chunk-29-1.png} |  Bin
 ...-knitr-markdown-rendered-rmd_to_html_fig-1.png} |  Bin
 md5sum.txt                                         |   51 +-
 reference.md                                       |   17 +-
 renv.lock (gone)                                   | 1043 --------------------
 16 files changed, 36 insertions(+), 1694 deletions(-)
What does this mean?

If you have source files that require output and figures to be generated (e.g. R Markdown), then it is important to make sure the generated figures and output are reproducible.

This output provides a way for you to inspect the output in a diff-friendly manner so that it's easy to see the changes that occur due to new software versions or randomisation.

⏱️ Updated at 2024-04-11 23:59:45 +0000

github-actions bot pushed a commit that referenced this pull request Feb 15, 2024
Copy link
Contributor

@skanwal skanwal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @milanmlft for tackling this.

I am happy with removing references to plyr.

@naupaka - can you please have a second look to confirm and merge this as I notice you've had a look at this issue back in 2019-2020?

@naupaka
Copy link
Member

naupaka commented Feb 22, 2024

I agree it's time to get this removed -- I am still wondering if there is a way to archive it or something. @tobyhodges is that something other lessons have done? If not, then I guess it is reasonable just to remove it and the references to it. I think it's reasonable to separate the removal of plyr and the request for a purrr lesson (which are both part of the discussion on #540).

@tobyhodges
Copy link
Member

tobyhodges commented Feb 23, 2024

@naupaka we could archive it e.g. by relocating it to the More dropdown menu: making it somewhat similar to the extra episode available in the DC Image Processing curriculum. What would be the motivation for keeping it, as opposed to removing it from the lesson altogether? My interpretation of the discussion is that plyr has been retired, and the skills and concepts taught in the episode are covered sufficiently elsewhere.

Good point about the request for purrr in #540. If/when this is merged, perhaps we can open a new issue to track that aspect? Or keep #540 open but rename it and add a note that plyr has been removed?

@milanmlft
Copy link
Contributor Author

milanmlft commented Feb 23, 2024

Good point about the request for purrr in #540. If/when this is merged, perhaps we can open a new issue to track that aspect? Or keep #540 open but rename it and add a note that plyr has been removed?

I'd suggest creating a new issue for this. Currently this PR is set to close #540 when it gets merged in.
That being said, I would argue that purrr is already a more advanced library and maybe not suitable for a novice R workshop? But that's just my view :)
(I guess this is the sort of discussion that could be held in the new issue)

@tobyhodges
Copy link
Member

I've opened the pleasingly-numbered #888 to provide a place for further discussion of the possibility to include an episode about purrr in the lesson.

@swcarpentry/r-novice-gapminder-maintainers is this ready to merge, or did you want to request something more from @milanmlft?

@skanwal
Copy link
Contributor

skanwal commented Apr 11, 2024

Happy for this to be merged. Thanks @tobyhodges and @milanmlft

github-actions bot pushed a commit that referenced this pull request Apr 12, 2024
@skanwal skanwal merged commit 3c63857 into swcarpentry:main Apr 12, 2024
3 checks passed
github-actions bot pushed a commit that referenced this pull request Apr 12, 2024
Auto-generated via {sandpaper}
Source  : 3c63857
Branch  : main
Author  : Sehrish Kanwal <[email protected]>
Time    : 2024-04-12 01:42:51 +0000
Message : Merge pull request #885 from milanmlft/milanmlft/540-remove-plyr-episode

Remove plyr episode and update episode numbering
github-actions bot pushed a commit that referenced this pull request Apr 12, 2024
Auto-generated via {sandpaper}
Source  : e5c1d03
Branch  : md-outputs
Author  : GitHub Actions <[email protected]>
Time    : 2024-04-12 01:45:31 +0000
Message : markdown source builds

Auto-generated via {sandpaper}
Source  : 3c63857
Branch  : main
Author  : Sehrish Kanwal <[email protected]>
Time    : 2024-04-12 01:42:51 +0000
Message : Merge pull request #885 from milanmlft/milanmlft/540-remove-plyr-episode

Remove plyr episode and update episode numbering
@milanmlft milanmlft deleted the milanmlft/540-remove-plyr-episode branch April 12, 2024 07:59
@naupaka
Copy link
Member

naupaka commented Apr 12, 2024

Wow, end of an era. plyr, we knew ye well.

Thanks all!

github-actions bot pushed a commit that referenced this pull request Apr 16, 2024
Auto-generated via {sandpaper}
Source  : 3c63857
Branch  : main
Author  : Sehrish Kanwal <[email protected]>
Time    : 2024-04-12 01:42:51 +0000
Message : Merge pull request #885 from milanmlft/milanmlft/540-remove-plyr-episode

Remove plyr episode and update episode numbering
github-actions bot pushed a commit that referenced this pull request Apr 16, 2024
Auto-generated via {sandpaper}
Source  : b3e4049
Branch  : md-outputs
Author  : GitHub Actions <[email protected]>
Time    : 2024-04-16 00:13:34 +0000
Message : markdown source builds

Auto-generated via {sandpaper}
Source  : 3c63857
Branch  : main
Author  : Sehrish Kanwal <[email protected]>
Time    : 2024-04-12 01:42:51 +0000
Message : Merge pull request #885 from milanmlft/milanmlft/540-remove-plyr-episode

Remove plyr episode and update episode numbering
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove plyr dependency
4 participants