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

Data Patch Pipeline and Patch 001 #69

Merged
merged 6 commits into from
Jul 30, 2024
Merged

Data Patch Pipeline and Patch 001 #69

merged 6 commits into from
Jul 30, 2024

Conversation

jikaczmarski
Copy link
Collaborator

This pull request incorporates a data patching methodology and a restructuring of the data folder. It also removes unused files and reorganizes things.

The web book builds great on my local machine and all figures and numbers work as expected.

In this PR is a data patch as well data/patches/patch_001 that adds the missing Southcentral Power Project data from 2013-2020.

@jikaczmarski jikaczmarski changed the title PatchesThis pull request incorporates a data patching methodology and a restructuring of the data folder. It also removes unused files and reorganizes things. The web book builds great on my local machine and all figures and numbers work as expected. In this PR is a data patch as well data/patches/patch_001 that adds the missing Southcentral Power Project data from 2013-2020. Data Patch Pipeline and Patch 001 Jul 29, 2024
@eldobbins
Copy link
Collaborator

What is the relationship between data/main.R and scripts/main.R?

@jikaczmarski
Copy link
Collaborator Author

What is the relationship between data/main.R and scripts/main.R?

data/main.R is a modern implementation of scripts/main.R. If we can confirm that the scripts directory has no dependencies, we might consider removing it.

@eldobbins
Copy link
Collaborator

OK. I don't think that needs to stand in the way of this PR. I will make an issue to clarify this in the future.
And thinking about it, it may make more sense to have the patches scripts in scripts/ but since the names of main.R conflict, that won't be possible until scripts/ is cleaned up.

(The justification for separating scripts and data is that scripts can be tracked in GitHub, but data really shouldn't. If they are separate, then data/ can be listed in .gitignore while scripts/ is tracked normally. Not really an issue with our tiny dataset - thinking of the future.)

#70

@eldobbins
Copy link
Collaborator

@ianalexmac this PR is OK with me (with the addition of the new issue) but I'm leaving approval to you because you are more familiar with the workflow.

@jikaczmarski
Copy link
Collaborator Author

@ianalexmac @eldobbins actually, the inline functions used to generate statistics are kept in scripts/inline_functions so there may be a case to maintain that or move it as well. Something to note, and I'll also add this note to the issue.

Copy link
Collaborator

@ianalexmac ianalexmac left a comment

Choose a reason for hiding this comment

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

Great work Jesse! Thank you.

@ianalexmac ianalexmac merged commit 0ce3d45 into acep-uaf:main Jul 30, 2024
ianalexmac added a commit to ianalexmac/aetr-web-book-2024 that referenced this pull request Jul 31, 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