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

Multiple constructors? / Separation of S7 objects and constructors #522

Open
eitsupi opened this issue Jan 18, 2025 · 2 comments
Open

Multiple constructors? / Separation of S7 objects and constructors #522

eitsupi opened this issue Jan 18, 2025 · 2 comments

Comments

@eitsupi
Copy link

eitsupi commented Jan 18, 2025

Thank you for pushing this wonderful project forward. It looks very promising.

I've read the documentation, but in the current implementation, S7 class instances can only exist via the single constructor?
I have tried to imagine creating a wrapper object for an external pointer (specifically something like Series in the polars package) in S7.

For example, the constructor must take a name (string) and values (R vector) and construct an external pointer stored in _s. (Note: Because it mimics Python's polars package)
The name can be changed later so it becomes a property.

s_foo <- Series(name = "foo", values = 1)

s_foo@name
#> foo
s_foo@`_s`
#> <external pointer>

# The name can be changed
s_foo@name <- "bar"
s_foo@name
#> bar

# The pointer should be frozen
s_foo@`_s` <- other_pointer
#> Error

But apart from this, it also requires an (internal-only) function to create objects by directly setting an external pointer.
Is it possible to create such a thing?
Ideally, it would be called a class method or an associated function (related to #202, #436), but it does not have to be.

# In Python Polars, there are two class methods to construct Series
Series._from_pyseries() # From Rust / Rust Polars' Series
Series._import_from_c() # From C / Apache Arrow C interface
# In R, something like these:
Series@.from_rseries()
Series@.import_from_c()

# These internal functions are also fine
series_from_rseries()
series_import_from_c()

I want to export the S7 object itself (Series) for use in the convert function, etc., but do not want to export the constructor for manipulating the external pointer.
Therefore, I do not want the default constructor to be one that takes an external pointer.

# The S7 object should be exported
Series
# Because it will be used with the `convert` function
convert(1, Series)
convert(a_nanoarrow_array_stream, Series)

# But I don't want to expose the constructor that builds the Series from an external pointer something like this
Series(.data = rust_series)
@t-kalinowski
Copy link
Member

Thanks for writing, these are excellent questions.

Adding support for multiple constructors might make sense.

However, I don't think the situation you describe requires them; it sounds like what you want can be accomodated by handling different values in the constructor. Consider:

Series <- new_class("Series", properties = list(
  name = class_character,
  .ptr = new_property(
    setter = function(self, value) {
      if (!is.null(self@.ptr)) {
        # warn, not error, until https://github.com/RConsortium/S7/issues/520 is fixed
        warning("changing .ptr value not permitted. This will error in the future.")
      }
      self@.ptr <- value
      self
    },
    validator = function(value) {
      if (typeof(value) != "externalptr")
        "must be an externalptr"
    }
  )),
  constructor = function(name, values) {
    if (is_rust_series(values))
      .ptr <- ...
    else if (is_arrow_c_series(values))
      .ptr <- ...
    else if (is.atomic(values))
      .ptr <- ...
    else
      stop("values must be ....")
    new_object(S7_object(), name = name, .ptr = .ptr)
  })

Worth noting, there is also the escape hatch of setting attributes directly, to bypass all setters and validator.
This is firmly in the "Only if you know what you're doing" territory, but I think it's a valid pattern for sophisticated projects:

series_import_from_c <- function(...) {
  # instantiate a prototype object, then modify it as needed.
  proto <- Series("", NULL)
  attr(proto, ".ptr") <- ...
  attr(proto, "name") <- ...
  validate(proto)
  proto
}

@eitsupi
Copy link
Author

eitsupi commented Jan 18, 2025

Thanks for the detailed answer!

However, I don't think the situation you describe requires them; it sounds like what you want can be accomodated by handling different values in the constructor.

True, but it seems rather odd to do if-branching inside an S7 object that promotes generic functions.

The workaround of directly modifying the object seems useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants