Proposal: get rid of assertthat and replace with unified argument checking/adapting framework w/rlang #262
Comments
|
Oh, I think it would be GREAT if you reworked all that asserting and checking! I think it would be a huge improvement. |
|
Okay, I'm working on it. BTW, how do you feel about using the new native pipes (|>)? It's a lot faster to write with them, but I wanted to run it by you first. |
|
Unfortunately not in a package yet, I think, since there are lots of people still using pre-4.1.0 R in their workplaces, servers, etc. It is a lot easier to update packages than R for many people. |
juliasilge
added a commit
that referenced
this issue
Jun 20, 2022
…223, & #262 (#263) * Refactoring of argument checking * functions use new arg checking, include_* + date/times in fetch_survey, extend fetch_id() * update tests * mark new arg checks as internal functions * updated fetch_survey roxygen * Further modularizing export-resposes (to support fetch_in_progress()) * documentation of new functions & fix inappropriate globals * documentation of new functions & fix inappropriate globals * remove extra param from qualtrics_response_codes * Use rlang::arg_match for more safety * No more assertthat * Use sort of glue-ish pseudocode * Fix order to generate protocol message, plus attempt some more consistency in messages * Redocument * Update tests * One more test * Update NEWS Co-authored-by: Julia Silge <julia.silge@gmail.com>
|
Glad that it helped. Closing this, but feel free to reopen if there's more to do. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
So, I'm trying to finish adding a couple of the really essential features like filtering (#223), time/date control (#158), and survey activation/deactivation (#215, since that's not much extra work once we have date/times going).
It's all pretty straightforward overall, but I keep running into how complicated our current handling of argument checking is making this. Right now:
assertthat, (in assertions.R).assert_*function.construct_raw_payload.Rasserthatis dead--it's not been updated in 5 years. There are other good options likecheckmate, but really as some of the recent updates have shown,rlang::abort()does quite well. Also it seems evident that any further development of this kind of thing from the hadley/tidyverse is coming from there (e.g., r-lib/rlang#1111).So, maybe continuing with
rlangmight make sense. But more generally while I'm trying to finish all this I was wondering if people wouldn't mind me reworking things more generally into streamlined approach that handles both cleaning and checking arguments in a unified way. It's probably more efficient for what I'm doing right now and would make everyone's efforts easier going forward. Thoughts? If people don't hate the idea I'll send a draft in shortly, and can adjust the strategy from there.The text was updated successfully, but these errors were encountered: