[r] Docs and NAMESPACE cleanup

[aaronwolen]

The goal of this PR is to reduce the number of exported functions and cleanup our documentation index to make it more user friendly.

I'm very open to feedback on this PR with regard to what should be exported and what should be internal. As per our guidelines for exporting functions I'm unexporting low-level functions that I don't think users would/should use directly. We can always decide to export more classes and functions in the future but it is harder to remove them once they are exported. So let's start with a minimal set of functions.

This gives us more flexibility to change the internal implementation of the package without breaking user-facing APIs.

Internal functions

Functions that are no longer exported:

• soma_array_reader
• sr_setup
• sr_next
• sr_complete
• pad_matrix
• nnz (this is a method of SOMASparseNDArray)
• shape (this is a method of TileDBArray and its child classes)
• libtiledbsoma_version() / tiledb_embedded_version() (I can definitely see a use case for exporting these so users can programmatically check the versions of libtiledbsoma and tiledb embedded but for now users can get all necessary version information from show_package_versions())

These functions were not exported but were generating Rd files. @mojaveazure pointed out that CRAN requires all functions with an associated Rd file to be exported, so they have been tagged with @noRd to prevent them from generating Rd files.

• soma_array_to_arrow_table
• arrow_table_to_dense
• arrow_table_to_sparse

Low-level base classes

These classes are all exported and marked as internal so they aren't advertised to users. The corresponding Rd files are still generated for developers but they will not appear in the documentation index.

Importantly their methods will still appear in the documentation of higher-level classes that inherit from them.

• TileDBObject
• TileDBArray
• TileDBGroup
• SOMACOllectionBase
• EphemeralCollectionBase
• SOMAArrayBase
• SOMANDArrayBase
• MappingBase
• ScalarMap
• SOMAContextBase
• ReadIter
• SOMASparseNDArrayRead
• SOMAAxisIndexer (previously unexported)

Ideally we would not export these classes at all but since CRAN requires Rd-generating classes/functions to be exported it would seem our hands are tied.

Other changes:

The following functions are now documented in a single Rd file:

• tiledbsoma_stats_disable()
• tiledbsoma_stats_dump()
• tiledbsoma_stats_enable()
• tiledbsoma_stats_reset()
• tiledbsoma_stats_show()

Notes for Reviewer: I had also planned to cleanup the pkgdown site as well but I'm going to save that for a subsequent PR.

[mojaveazure]

The @export tags are (IIRC) still needed for the method definitions to enable S3 method dispatch (@export on a function vs @export on a method are different)

[mojaveazure]

(looking at the diff for NAMESPACE shows that the method definitions are no longer registered)

[eddelbuettel]

I have @export on a ton of methods in tiledb-r too. Plus occasional exportClass.

I think roxygen2 behavior changed here over the years.

(Usual disclaimer: roxygen2 is something "we" (as in R users) do, it is not normative or even mentioned really in WRE. Only the final NAMESPACE and man/ matter.)

[eddelbuettel]

Note that roxygen seems to do the right thing here and aggregates under the method. One file man/pad_matrix.Rd generated. For your ease of reading I have:

% Generated by roxygen2: do not edit by hand
% Please edit documentation in R/pad_matrix.R
\name{pad_matrix}
\alias{pad_matrix}
\alias{pad_matrix.matrix}
\title{Pad a Matrix}
\usage{
pad_matrix(x, ...)

\method{pad_matrix}{matrix}(x, rowidx, colidx, shape, sparse = FALSE, ...)
}
\arguments{
\item{x}{A matrix-like object}

\item{...}{Arguments passed to other methods}

\item{rowidx, colidx}{Integer indices for rows and columns from
\code{x} to \code{shape}}

\item{shape}{Target shape to pad out to}

\item{sparse}{Return a \link[Matrix:TsparseMatrix-class]{sparse matrix}}
}
\value{
A matrix containing the data of \code{x} padded out to \code{shape}
}
\description{
Pad a Matrix
}
\keyword{internal}

[eddelbuettel]

(Disclaimer: It is correct if one wants the method public. )

[aaronwolen]

@mojaveazure are you suggesting S3 dispatch for pad_matrix() will not work if it's not publicly exported?

[mojaveazure]

Yes; I've had issues where S3 dispatch fails when the methods are not exported. Happy to test this out more if need be

[aaronwolen]

I made a quick toy example to test and it doesn't appear to be a problem. Check it out and let me know if I've missed anything: https://github.com/aaronwolen/reverse

[mojaveazure]

Using @aaronwolen's example package, it seems as though S3 dispatch works as long as usage is within-namespace. The moment we call the generic from outside of the namespace, we lose dispatch. I think this is acceptable behavior for pad_matrix()

[eddelbuettel]

Isn't this a non-issue? Once we decide to not export it, we pretty much say it won't (or may not) work outside the package -- and we care mostly about the use inside the package. So ... onwards!

Feared loss of functionality proved untrue

[eddelbuettel]

We may have used Title Style on purpose. But we are possibly not quite consistent across so ...

[eddelbuettel]

With that the caveat that it is a long PR -- long good. Thanks for doing that.