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

Consider eliminating string representation of base types #170

Closed
hadley opened this issue Feb 14, 2022 · 5 comments · Fixed by #207
Closed

Consider eliminating string representation of base types #170

hadley opened this issue Feb 14, 2022 · 5 comments · Fixed by #207

Comments

@hadley
Copy link
Member

hadley commented Feb 14, 2022

In favour of base_integer, base_numeric, etc.

Suggested by @lawremi in February meeting.

@hadley
Copy link
Member Author

hadley commented Feb 14, 2022

Also consider providing canned definitions of most important S3 classes using the same naming convention.

@hadley
Copy link
Member Author

hadley commented Feb 16, 2022

One thing that's not create about looking up by constructor is that since identical(double, numeric) is true, then as_class(numeric) returns a double.

Another principle I think we're circling in on is that there should be exactly one way to specify a class.

@hadley
Copy link
Member Author

hadley commented Feb 22, 2022

(Summary of a discussion with @DavisVaughan)

It's worth considering the naming scheme holistically. Currently as_class() supports:

  • Base constructors: logical, integer, double, character etc
  • Names of base types: "logical", "integer", "double", "character" etc
  • Names of base unions: "numeric", "atomic", "vector".
  • Special sentinels missing_class and any_class
  • Inline S3 classes: new_S3_class("factor")

And we probably also want to provide R7 wrappers for the most important S3 classes (e.g. factor, Date, POSIXct/POSIXlt, data.frame).

I think ideally we want a single way of specifying each class, so that R7 code varies as little as possible from person-to-person. It's hard to make this work with base constructors because double and numeric are identical, and atomic doesn't exist. It would be nice to have a consistent prefix so that autocomplete can remind you of the options.

A few ideas:

  • base_ works quite nicely as a prefix (base_integer, base_atomic, base_factor), but would be inconsistent with the _class suffix for any_class and missing_class (but maybe that's ok).

  • class_ also seems ok (class_integer, class_atomic, class_factor), but would imply class_any and class_missing which don't feel quite right.

  • I think we want to avoid R7_ as a prefix because R7_integer feels like it represents a special R7 integer class.

@lawremi
Copy link
Collaborator

lawremi commented Mar 8, 2022

I was also thinking that class_ would be a good prefix, because it is much more descriptive than base_. class_any and class_missing seem OK to me. After all, they were specified in the same way as classes in S4.

@hadley
Copy link
Member Author

hadley commented Mar 17, 2022

Having implemented this, I really like it. The autocomplete after typing class_ makes it very clear what your options are, which was never explicit before.

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.

2 participants