nhs-r-community / demos-and-how-tos

A repo for community contributed demos and how-tos to get common stuff done in the R language
https://nhs-r-community.github.io/demos-and-how-tos/
MIT License
29 stars 13 forks source link

Camcoder #54

Closed Pablo-source closed 1 year ago

Pablo-source commented 1 year ago

This is an example on how to create animated GIF using Camcoder package

Pablo-source commented 1 year ago

Added all Camcoder files to the original ggplot2-visualizations project. Hence closing this comment as all the files are up to date.

Pablo-source commented 1 year ago

Keep it open because I want it to be merged with the main demos-and-how-to

ChrisBeeley commented 1 year ago

This is great, thank you! I'm wondering if we should .gitignore the .png files to clean up the PR a bit, any thoughts @Lextuga007 @tomjemmett ?

tomjemmett commented 1 year ago

I would typically argue that items that are generated by code, e.g. these .png's should be .gitignore'd as they can always be generated again, and as such should be rendered in an action when gh-pages are deployed... that said, we currently aren't utilising actions to deploy the content.

one thing to note is git repo's can get bloated very quickly with lot's of binary files, at a very quick glance there appears to be ~10 .png's at ~150Kb, and one .gif at ~1Mb.

so I think we should exclude these, then do a squash merge (that should drop these file from the commit history)

ChrisBeeley commented 1 year ago

Is that OK @Pablo-source ? We can do the changes for you or you can resubmit if you prefer. Not to be pedantic but it's good to keep the repo clean 🧹

Pablo-source commented 1 year ago

Hi @ChrisBeeley and @tomjemmett many thanks for your suggestions, I am going to include in the .gitignore all the .png files as both suggested. I will do one commit today with this change to exclude those files produced by camcoder. I will keep only the R script and the GIF output file produced if that is fine.

Pablo-source commented 1 year ago

Many thanks @ChrisBeeley and @tomjemmett . Learning a log on this first pull to this repo. Ammending right now the .gitignore file to exclude all .png filles and I will commit it again in the next 40 minutes.

ChrisBeeley commented 1 year ago

Many thanks @ChrisBeeley and @tomjemmett . Learning a log on this first pull to this repo. Ammending right now the .gitignore file to exclude all .png filles and I will commit it again in the next 40 minutes.

Great to hear- that's what NHS-R is all about- learning by doing. Thanks for the PR 🤗

Pablo-source commented 1 year ago

HI @ChrisBeeley and @tomjemmett this should be fixed now. Created a new .gitignore file at the ggplot2-visualizations folder level, that refers to the Camcoder subfolder to ignore all .png files. I tried first to list them individually. It should work now, please let me know otherwise.

demos-and-how-tos/ggplot2-visualizations/.gitignore

.gitignore content (among other files)

/Camcoder/2023_02_23_09_08_00.422555.png /Camcoder/2023_02_23_09_08_00.678560.png /Camcoder/2023_02_23_09_08_00.934180.png /Camcoder/*.png

ChrisBeeley commented 1 year ago

I've tidied this up into a new PR #55 . Sorry I should have done it properly really.

@Pablo-source , please take a look and see what you think. I've requested a review from @Lextuga007

ChrisBeeley commented 1 year ago

Is this PR finished and ready for review?

Lextuga007 commented 1 year ago

Yes, but @Pablo-source has made some changes to his own branch and this needs to be merged into that. I tried merging your branch to Pablo's locally but got merge conflicts on the data files.

Lextuga007 commented 1 year ago

Resolved with #55