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

Restructure everything to a package #110

Open
lboel opened this issue Dec 13, 2023 · 4 comments · May be fixed by #121
Open

Restructure everything to a package #110

lboel opened this issue Dec 13, 2023 · 4 comments · May be fixed by #121
Assignees
Labels
enhancement New feature or request

Comments

@lboel
Copy link
Collaborator

lboel commented Dec 13, 2023

Here I will lay out the steps to convert the current code base to a relatively clean package structure:
First Version:

  • Just move scripts/script<1-3> to functions in /R folder and make the runner script accept something like: a4d::step_1,a4d::step_2,a4d::step_3
  • change our workflow to call devtools::install() instead of load_all()
@lboel lboel self-assigned this Dec 13, 2023
@lboel lboel added the enhancement New feature or request label Dec 13, 2023
@lboel
Copy link
Collaborator Author

lboel commented Dec 13, 2023

@pmayd

@pmayd
Copy link
Collaborator

pmayd commented Dec 13, 2023

@lboel but we have to replace all function calls with a4d::, have we not? Like the fix functions etc. So basically everywhere where we were able to call a function from the /R folder because it was available thanks to load_all should now be called with a4d:: in front of it, or not? Or is this not necessary because we are inside the package and this is only relevant like in the main script where I want to call a function from outside?

@pmayd
Copy link
Collaborator

pmayd commented Dec 21, 2023

@lboel what do you think about changing the main functions into handling a single file instead of inside getting a list of files?
I think having three main steps as a pipeline for a SINGLE file will simplify the code and we can more easily test a single file with the pipeline by just executing this function.
This would mean to refactor at least the two first scripts to accept a file and move the list.files functionality outside and iterate over the list from outside. This is also in preparation of parallelising the loops more easily as we can directly use parallel loop versions...

I guess this comment is not really helpful after all as I think we need to do this anyway and this was the idea from the beginning but yeahg...

pmayd added a commit that referenced this issue Dec 28, 2023
…AidSwitzerland/a4d into refactor/#110-make-proper-package
@pmayd pmayd linked a pull request Jan 2, 2024 that will close this issue
@pmayd
Copy link
Collaborator

pmayd commented Jan 24, 2024

Decided to put this on hold for now in favor of more high priority issues

  • have to fix log problems first before going forward with this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants