Feature Request: conditions should be classed

[bms63]

Feature Idea

See pharmaverse/admiral#1319 (comment) for discussion

• Should be worked on simultaneously on related admiral issue
• First step is identifying the functions that need to be updated

Relevant Input

No response

Relevant Output

No response

Reproducible Example/Pseudo Code

No response

[zdz2101]

For the most part, many of our assertions' abort() messages can be categorized into a few reasons:

1. "must be" statements where the argument is of the wrong data-type, usually encased in a is.xxx() if bracket
2. "missing"-ness where the argument/element/variable is missing, usually encased in a is_missing() if bracket
3. "unique"-ness where generally the function unique() is being called somewhere in the function body and its going to give us grief

My proposal at least for admiraldev if we want to move forward and implement classed conditions would be to adopt at least 3 classes for errors/abort-messaging:

1. assertion_type_error
2. assertion_missingness_error
3. assertion_uniqueness_error

@pharmaverse/admiral I do wonder is this something we absolute need as a core-dev team right now and although for the most part adding these classes can only help, I'm not sure the target demographic - SAS turned R programmers are going to be running too much tryCatch() and withCallingHandlers() in their troubleshooting. As for us, I'm not sure how often we do either, perhaps I can send out a survey of some sort (?).

Prioritizing something like #277 might help us a lot more depending where we all stand on debugging as a general topic and perhaps a debugging vignette is directionally more aligned to what we want right now*?

[zdz2101]

Of course implementing these classed conditions comes with the task of properly creating the documentation for it somewhere which is kinda another point of discussion in this broad-based topic of debugging

[bms63]

I would be more interested on learning more on how to use the debugging tools properly and then focus on classes at a later time. I do like your summary for the types of classes.

While not trying to bury anything - it might be more fitting to move this to a discussion thread and move this into discussion at our core meetings.

[bundfussr]

I think there is a misunderstanding. The goal is not to create classes which cover a set of similar conditions. On the contrary, the classes should enable us to uniquely identify the condition which was not fulfilled. I see two use cases:

• In unit tests we can check on the class instead of the error/warning message. We do this already for deprecation tests. Then we do not need to update the tests if the wording of the message changes.
• The classes can be used to provide better messages to the user. For example, if a derive_*() function calls filter_extreme() with arguments which were not provided directly by the user, the error class issued by filter_extreme() can be catched and a more user friendly message (referring to the arguments provided by the user) can be created.

See also https://adv-r.hadley.nz/conditions.html#custom-conditions.

I agree that we do not need this now. I think the clean-up has higher priority.

[bms63]

CAn we go ahead and move this and related issues to a discussion thread and pick this up at a later time?

[zdz2101]

Migrated the admiral issue into a discussion

[bms63]

CLosing this and linking in discussion pharmaverse/admiral#2022