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

Add patch option to get_script_fun #207

Open
crich011 opened this issue Sep 3, 2019 · 4 comments
Open

Add patch option to get_script_fun #207

crich011 opened this issue Sep 3, 2019 · 4 comments
Milestone

Comments

@crich011
Copy link

crich011 commented Sep 3, 2019

get_script_fun admits post and list as arguments but not yet patch. This would need to add logic to ensure the user doesn't try to get a function like scripts_patch_containers_runs, which doesn't exist.

@patr1ckm
Copy link
Contributor

patr1ckm commented Sep 4, 2019

Yes. I think adding a runs argument would handle this, and allow fetching of more script related functions like scripts_post_containers.

@crich011
Copy link
Author

crich011 commented Sep 4, 2019

Ah, so are you imagining adding runs as an argument option to fun_type along with adding patch as an argument option to verb, for example? In such a case, what do you imagine as the trade-off between client-end vs. user-end checking to ensure that the function it returns is valid?

For example, would trying to verify with something like exists("scripts_post_containers", where = "package:civis", mode = "function") work? (That seems like a simpler and better approach than the hacky series of asserts that I had in mind.)

@patr1ckm
Copy link
Contributor

patr1ckm commented Sep 5, 2019

Ah, so are you imagining adding runs as an argument option to fun_type along with adding patch as an argument option to verb, for example?

Yes!

what do you imagine as the trade-off between client-end vs. user-end checking to ensure that the function it returns is valid?

Great q. The get call in get_script_fun returns the function in the package if it's available, and fails otherwise. Your suggestion using exists works similarly! Since the user of the function is the developer, it's on the developer to make sure that usage checks out. An end-user will experience a failure here as a cryptic error, but is a bug.

@crich011
Copy link
Author

crich011 commented Sep 5, 2019

Ahh, cool! I didn't realize that get did that! (Here I was imagining some complicated set of checks and it was already happening right under my nose!)

@patr1ckm patr1ckm added this to the 2.2 milestone Sep 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants