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

Dispatching on "missing" #67

Closed
lawremi opened this issue Jun 1, 2021 · 12 comments · Fixed by #177
Closed

Dispatching on "missing" #67

lawremi opened this issue Jun 1, 2021 · 12 comments · Fixed by #177

Comments

@lawremi
Copy link
Collaborator

lawremi commented Jun 1, 2021

S4 lets us dispatch on "missing", i.e., an argument in the signature not being provided. This is a somewhat useful feature for multiple dispatch. Should/could we support that in R7?

@hadley
Copy link
Member

hadley commented Jun 1, 2021

Is there an advantage over providing NULL as a default value (which makes it clear that the argument is indeed optional) and then dispatching on that?

@lawremi
Copy link
Collaborator Author

lawremi commented Jun 1, 2021

One example is [(), where x[i,] is not the same as x[i,NULL]. Another is IRanges::findOverlaps(pattern, subject), where omitting subject computes overlaps of pattern with itself. Overlaps with NULL might intuitively have a different semantic (no overlaps). Making subject default to pattern does not work either, because the missing method has arguments past ... that only make sense in a self comparison. There are of course work arounds, but dispatching on missing has proven convenient.

@mmaechler
Copy link
Collaborator

I strongly agree with @lawremi : R like optional arguments make sense a lot. Indexing (and subsetting, i.e., the [<- generic) semantics in S and R need this distinction (and the method implementation in some of my cases in Matrix then may additionally use nargs() to distinguish cases.. because there you also need to distinguish x[i,] from x[i].

@hadley
Copy link
Member

hadley commented Jun 2, 2021

I think this is a confusing API (since you can't tell from the function formals which arguments are required and which are optional), but it seems like we'll need it for backward compatibility.

@lawremi
Copy link
Collaborator Author

lawremi commented Jun 2, 2021

Having this information gives us the opportunity to automatically clarify argument handling in the generic documentation, so that we do not have to depend on the formals and the presence of a default value to communicate optionality.

@hadley
Copy link
Member

hadley commented Jan 31, 2022

Any syntax preferences for this? We could use "missing" or we could make it a special symbol (e.g. missing_arg).

(It looks like method dispatch for missing arguments is already implemented, but method registration is not.)

@lawremi
Copy link
Collaborator Author

lawremi commented Feb 2, 2022

For consistency, what about making something like missing_arg be a class object, representing a dummy virtual, final class? That might even simplify the implementation?

@hadley
Copy link
Member

hadley commented Feb 2, 2022

Yeah, sorry that's what I meant by "special symbol". Does missing_arg seem like the right name? Another option would maybe be r7_missing? We could also make missing work, although it might be a bit confusing since we'd just be using the existing function as a sentinel (like we do for character, integer etc, but there the connection is much closer).

@lawremi
Copy link
Collaborator Author

lawremi commented Feb 9, 2022

missing_arg seems good enough for now at least.

@hadley
Copy link
Member

hadley commented Feb 13, 2022

Also need to implement/test dispatch on "ANY", which I think might have gotten muddled up with dispatch on NULL.

@hadley
Copy link
Member

hadley commented Feb 17, 2022

What do you think of "absent" instead of "missing"? It avoids some of the confusion associated with NA.

And I think maybe "_class" works better than arg, since we'll want to write new_property("foo", class = any_class).

@hadley
Copy link
Member

hadley commented Feb 17, 2022

I've gone with missing_class and any_class for now; if needed we can revisit when we look holistically at all the built in class names.

hadley added a commit that referenced this issue Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants