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

adding english-monarchs-and-marriages (enmoma) dataset #721

Merged

Conversation

frankiethull
Copy link
Contributor

creating the english-monarchs-and-marriages branch, aka enmoma for short. testing the pull request method with tidytuesday team on this old issue 480.

one thing to note, I did not use ttsave()!, I'm not sure if that function exists or if I'm missing a step, please review saving.R script.

…ort. testing the pull request method with tidytuesday team on this old issue
Copy link
Collaborator

@jonthegeek jonthegeek left a comment

Choose a reason for hiding this comment

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

Please try to run the source() line in saving.R, and let me know if that doesn't work. It SHOULD give you the ttsave() function, which (in addition to saving the dataset) creates the data dictionary and runs some (minimal for now but more to come) checks.

We clearly need to make this instruction clearer in any case!

data/curated/enmoma/saving.R Outdated Show resolved Hide resolved
Copy link
Collaborator

@jonthegeek jonthegeek left a comment

Choose a reason for hiding this comment

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

I can't give this a full review right now, so I'm sending this single comment. If you don't have a chance to update things right away, no worries. I'll try to dig into the rest of this submission sometime tomorrow.

Thank you so much for testing this process! You're the first person to learn the process purely from the instructions, and there are clearly some things for us to clarify!

data/curated/enmoma/english_monarchs_marriages_df.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@jonthegeek jonthegeek left a comment

Choose a reason for hiding this comment

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

Just a couple tweaks needed. I could just implement themselves, but I wanted to check with you about the credit (eg, if you'd like to include an affiliation or anything along those lines).

Thank you so much for being our Guinea pig!

data/curated/enmoma/meta.yaml Outdated Show resolved Hide resolved
data/curated/enmoma/intro.md Outdated Show resolved Hide resolved
@lgibson7
Copy link
Member

Yay! Our first (correct me if I'm wrong @jonthegeek) externally curated Tidy Tuesday data set. Thank you so much @frankiethull. I truly appreciate your help with this.

@jonthegeek
Copy link
Collaborator

Yay! Our first (correct me if I'm wrong @jonthegeek) externally curated Tidy Tuesday data set. Thank you so much @frankiethull. I truly appreciate your help with this.

Not first ever, but first using the new, formal system. The one or two we've had in the past ended up needing a lot of manual edits to fit the format, since we didn't have any guidance on what to do. So this is still a very "yay!" moment!

@frankiethull
Copy link
Contributor Author

"yay!" moment confirmed over here!! Glad to be your guinea pig and that credit is perfectly fine with me.

I use the vertical bar "|" often so this is a helpful note for next time.

I had shared a handful of datasets back in 2022, which looks like they will all be closed issues now (woohoo!). But with this new process, I'll be on the lookout for other interesting datasets. I also saw you all did a rebranding as DSLC, congratulations, I have been out of the loop!

Additionally saw this, "We hope to provide instructions for other programming languages soon", I could always be a tester for Julia and Python too, let me know when and where 😤

@jonthegeek
Copy link
Collaborator

/assign enmoma 2024-08-20

Copy link
Collaborator

@jonthegeek jonthegeek left a comment

Choose a reason for hiding this comment

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

I have to commit one fix that I didn't catch (but it became obvious after the script to assign this to a week). Thanks again! Your contribution will appear as next week's dataset!

data/2024/2024-08-20/readme.md Outdated Show resolved Hide resolved
@jonthegeek jonthegeek merged commit 6c4903e into rfordatascience:master Aug 14, 2024
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.

3 participants