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

Consider moving map-related processing to pacta.portfolio.report #102

Open
AlexAxthelm opened this issue May 27, 2024 · 3 comments
Open

Comments

@AlexAxthelm
Copy link
Collaborator

AlexAxthelm commented May 27, 2024

Given that the has_map parameter is TRUE when an analysis is run for preparing a report, and FALSE otherwise, it makes sense that we could remove this parameter entirely, and move the relevant sections of the code to be part of the report generation process, rather than requiring it to be specified prior to knowing the end-use-case for an analysis.

The data dependencies seem pretty minimal (total weighted portfolio and the ABCD files from pacta-data).

Currently the has_map parameter controls flow in workflow.pacta:

https://github.com/RMI-PACTA/workflow.pacta/blob/c3376bef6c1f6ecfe17fbbe02eb40f5620a5a66b/R/calc_weights_and_outputs.R#L88-L109

https://github.com/RMI-PACTA/workflow.pacta/blob/c3376bef6c1f6ecfe17fbbe02eb40f5620a5a66b/R/calc_weights_and_outputs.R#L153-L163

cc @cjyetman @jdhoffa

@cjyetman
Copy link
Member

I guess this would have to be implemented in pacta.portfolio.report first before considering removing it from here.

@cjyetman cjyetman transferred this issue from RMI-PACTA/pacta.portfolio.allocate Dec 12, 2024
@jdhoffa
Copy link
Member

jdhoffa commented Dec 12, 2024

I'm so confused, here is pacta.portfolio.report? Are we moving it from here to here? lol

@jdhoffa
Copy link
Member

jdhoffa commented Dec 12, 2024

Ah sorry, just noticed @cjyetman moved the issue 🤣

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

No branches or pull requests

3 participants