Review convenience implementations for `vctrs_vctr`

[lionel-]

Currently unclear what are the consequences of the special handling of vctrs_vctr classes. It makes me nervous that rcrds and list-ofs inherit from the current behaviour that I don't understand well. I think we need a better and clearer foundation for the convenience implementations.

Should vctrs_vctr classes inherit from the base coercion diagram, i.e. if the base type is double it's coercible to integer and logical automatically? Should this be by default or opt-in? Should this be only for classes that don't implement any vec_ptype2() methods?

[lionel-]

I also wondered if explicit inheritance from a base type would be the way to inherit from the base coercion dag. With this approach the UI is a bit weird since classes normally don't inherit from coercion methods. The base class would be a confusing exception.

I think my preference goes to vctrs_vctr classes inheriting the coercion dag as long as they don't implement a ptype2 method. Once implemented, the full set of ptype2 and cast methods needs to be implemented.

[DavisVaughan]

I'm starting to think that it would be nice to remove all special casing of vctrs_vctr, vctrs_rcrd, and vctrs_list_of, and even remove any mentioning of them at the C level (except fields.c).

I think it should be possible for these classes to be implemented in a separate package outside of vctrs (I'm not saying we should, I just think that clear separation would be valuable).

I looked at the codebase, and I don't think it would be that hard. It would force us to make sure that we are providing all the correct customization points.

---

Here is what would have to be tweaked (it isn't much!):

• C level new_list_of() and init_list_of() are no longer used and can be removed.

  • This also means we can remove classes_list_of.

• The C type vctrs_class_list_of is only used in vec_is_list(), but if we remove it we will still get the right results because of vec_is_vector() (i.e. vec_proxy.vctrs_list_of exists, proof that we have the right customization point here).

  • That would let us remove strings_vctrs_list_of

• The C type vctrs_class_rcrd is used in vec_is_list() as a placeholder, but removing it still results in correct fallthrough behavior. It is also used in the C level is_record(), which is not used anywhere.

  • This would let us remove vctrs_class_posixlt too, but not vctrs_class_bare_posixlt.

  • This would let us remove strings_vctrs_rcrd

• The above two would let us remove strings_vctrs_vctr

And I think that is all of the C level usage of these vctrs types.

---

On the R side, the only "internal" usage of these vctrs types looks to be that special casing in vec_default_cast() to call vctr_cast(), which I think we should look hard at and consider removing.

[lionel-]

This issue is more about the ideal semantics needed for making vctrs_vctr a convenient class, not about the mechanism to implement it. The goal is eventually to figure out a general mechanism that can also be used by packages.

See also #989 about the idea of providing hooks in default methods. But I think this can only be a stopgap that will lead to inconsistent semantics.

[lionel-]

@DavisVaughan I like your proposal for removing the C-level special casing of these classes.

[lionel-]

There is no ptype2 default for vctrs_vctr objects, only for cast. This convenience default does two things:

1. If the object to coerce is classed, check it's the same type with is_same_type(). We already have a general fallback like this for any kind of classes so this is redundant.

2. If the object to coerce is unclassed, use vec_restore().

(1) is redundant and I think the usefulness of (2) will be limited once:

• The base class is inherited by default in new_vctr().
• The base class fallback is generalised (#1135)

After these two changes have been implemented, I think we should remove the vctrs_vctr special-casing in vec_cast().