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

deal with moving plot_match_success_rate() into r2dii.plot TODO #236

Closed
cjyetman opened this issue Nov 13, 2024 · 8 comments · Fixed by #238
Closed

deal with moving plot_match_success_rate() into r2dii.plot TODO #236

cjyetman opened this issue Nov 13, 2024 · 8 comments · Fixed by #238
Assignees

Comments

@cjyetman
Copy link
Member

# TODO: move to `r2dii.plot` and export
plot_match_success_rate <- function(data,

@MonikaFu is there still any realistic plan to move plot_match_success_rate() into r2dii.plot?

@jdhoffa
Copy link
Member

jdhoffa commented Nov 13, 2024

Either way, suggested actions here:

  • Open an issue in r2dii.plot with the suggestion
  • Remove the TODO from pacta.multi.loanbook@main
  • Close this issue

@cjyetman
Copy link
Member Author

now tracked in RMI-PACTA/r2dii.plot#574

@MonikaFu
Copy link

@cjyetman I have no visibility on this. Depending on how useful that would be we could do this. Decision up to @jdhoffa and @jacobvjk I would say.

@cjyetman
Copy link
Member Author

@cjyetman I have no visibility on this. Depending on how useful that would be we could do this. Decision up to @jdhoffa and @jacobvjk I would say.

@MonikaFu I have already removed the TODO comment from the code in this repo, and this issue will be closed when that PR merges.

The idea of moving plot_match_success_rate() into r2dii.plot has already been moved to RMI-PACTA/r2dii.plot#574, so it is now completely in your hands as the maintainer of that repo 😁

@jdhoffa
Copy link
Member

jdhoffa commented Nov 14, 2024

@cjyetman I have no visibility on this. Depending on how useful that would be we could do this. Decision up to @jdhoffa and @jacobvjk I would say.

My recommendation is to leave RMI-PACTA/r2dii.plot#574 open without action. That way the idea is tracked (in a better format than a TODO), but we don't need to action anything yet (which I see no reason to do in the short term)

@cjyetman
Copy link
Member Author

@cjyetman I have no visibility on this. Depending on how useful that would be we could do this. Decision up to @jdhoffa and @jacobvjk I would say.

My recommendation is to leave RMI-PACTA/r2dii.plot#574 open without action. That way the idea is tracked (in a better format than a TODO), but we don't need to action anything yet (which I see no reason to do in the short term)

RMI-PACTA/r2dii.plot#574 appears to have been a duplicate of RMI-PACTA/r2dii.plot#563 🤣 so I closed it (with appropriate comment and link) and left the original open

so just to be clear, this concept has now been moved to RMI-PACTA/r2dii.plot#563

@jacobvjk
Copy link
Member

I think there is an argument for making the match success rate available in r2dii.plot so that it can be used in individual analyses in low threshold way. I am thinking IFC applications for example. It would also be good to have something like that available if/when we set up a P4B cookbook. But definitely no urgency right now

@cjyetman
Copy link
Member Author

I think there is an argument for making the match success rate available in r2dii.plot so that it can be used in individual analyses in low threshold way. I am thinking IFC applications for example. It would also be good to have something like that available if/when we set up a P4B cookbook. But definitely no urgency right now

@jacobvjk maybe add that here RMI-PACTA/r2dii.plot#563

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 a pull request may close this issue.

4 participants