DCElement classes not listed when namespace not loaded

[mpadge]

This reprex demonstrates:

dcentry <- atom4R::DCEntry$new()
#> Loading Atom XML schemas...
#> Loading DCMI vocabularies...
dcentry$setId("my-dc-entry")
dcentry$addDCDate(Sys.time())
#> Error : $ operator is invalid for atomic vectors
#> Error in clazz$new: $ operator is invalid for atomic vectors

^{Created on 2022-02-22 by the reprex package (v2.0.1)}

The problem arises here:

atom4R/R/DCElement.R

Lines 62 to 63 in df577bc

	DCElement$getDCClasses = function(extended = FALSE, pretty = FALSE){
	list_of_classes <- unlist(sapply(search(), ls))

If the namespace is not loaded, none of the classes are found, and getDCClasses returns an empty list. A simple solution would be to insert something like:

if (!isNamespaceLoaded("atom4R")) {
  attachNamespace("atom4R")
}

That has undesirable side-effects like issuing packageStartupMessages, so maybe you can think of a better solution?

[mpadge]

Update to avoid loading namespace:

list_of_classes = unlist(sapply(search(), ls))
if (!"atom4R" %in% loadedNamespaces()) {
  list_of_classes <- c (list_of_classes, ls(envir = asNamespace("atom4R")))
}
# ... proceed with rest of fn

[eblondel]

I'll look at this asap and let you know

[eblondel]

From what I see, your proposal induces an atom4R import. I don't understand why you can't import atom4R namespace, especially if you intend to use atom4R as main dependency. To me that's not really a bug. It's intentional to have DC classes loaded when atom4R is loaded. atom4R is so far one of these R packages that need to be loaded to benefit of its capabilities.

[mpadge]

The problem is that "packages that need to be loaded to benefit of its capabilities" can then not be used as dependencies by any other packages. atom4R is currently not used by any other packages, and won't be able to be until this issue is fixed. Other packages which depend on atom4R really need to be able to do so without having the full namespace loaded.

[eblondel]

The method you try to use from your package was initially designed for internal use for atom4R functioning on encoding to DC xml model. What is initially intended to be used in external package is the R6 model on top of Dublin Core / DCMI terms and it is used in other packages. This said, I 've seen your test here: https://github.com/ropenscilabs/deposits/blob/a4f2fd36a670273d2f0943d11d3f8a3bf2ecfbc2/tests/testthat/test-metadata.R#L21 and I will look further into this issue to see what can be done to satify your needs.

[eblondel]

@mpadge why don't you use requireNamespace as suggested by the R CMD Check? Since you consider using extensively the atom4R R6 DC model, is it an issue?

[eblondel]

@mpadge FYI, atom4R is used in https://github.com/eblondel/geoflow See https://github.com/eblondel/geoflow/blob/master/R/geoflow_action_atom4R_dataverse_deposit_record.R and it is planned to be used in other packages in factory, including packages to federate approaches for cloud (meta)data upload. geoflow already covers part of this federating approach.

Again atom4R has been primarily designed to provides an XML implementation of the Atom and Dublin Core standards, and support standard Atom-based clients for (meta)data deposition. The listing of R6 classes designed in atom4R is done for the internal functioning of atom4R. Again as long as you follow CRAN package build recommendations, ie to use requireNamespace, in your code, you will get rid of the warning you have when running R CMD Check.

[mpadge]

The problem is not about warnings; the problem is that the package simply does not work when used as a dependency. These kinds of problems can and really should be fixed internally here. Using loadNamespace or requireNamespace do not fix the problem, and so the code you link to above from geoflow, which uses requireNamespace, is still inadequate. The only way to overcome the problems within this package are to attach the namespace to the search path:

requireNamespace ("atom4R") # same as `load;
#> Loading required namespace: atom4R
#> Loading Atom XML schemas...
#> Loading DCMI vocabularies...
isNamespaceLoaded ("atom4R")
#> [1] TRUE
grep ("atom4R", search ()) # This is the problem
#> integer(0)
dcentry <- atom4R::DCEntry$new()
dcentry$setId("my-dc-entry")
dcentry$addDCDate(Sys.time())
#> Error : $ operator is invalid for atomic vectors
#> Error in clazz$new: $ operator is invalid for atomic vectors

# And this is one example where this problem arises:
# Current R/DCElement.R#L63
list_of_classes <- unlist(sapply(search(), ls))
grep ("atom4R", names (list_of_classes))
#> integer(0)

^{Created on 2022-04-08 by the reprex package (v2.0.1)}

Using attachNamespace is the only way to overcome this. This is, however, an extreme imposition which many potential users, including myself, may be understandably unwilling to resort to. To quote Hadley

Of the four [ways of loading/attaching namespaces], you should only ever use two:

Use library(x) in data analysis scripts. It will throw an error if the package is not installed, and will terminate the script. You want to attach the package to save typing. Never use library() in a package.

Use requireNamespace("x", quietly = TRUE) inside a package if you want a specific action (e.g. throw an error) depending on whether or not a suggested package is installed.

Requiring anybody who wants to use atom4R to attachNamespace is problematic, and will prevent usage and adoption of this package. This is all fixable from within your package so that everything works without this imposition, but you don't seem very willing to implement any such fixes. As a final note,

The listing of R6 classes designed in atom4R is done for the internal functioning of atom4R

That's effectively a statement that the package is not intended to be used as a dependency by any other package.

[mpadge]

This is why using, let alone requiring, attachNamespace is not an option:

attachNamespace ("atom4R")
#> Loading Atom XML schemas...
#> Loading DCMI vocabularies...
attachNamespace ("atom4R")
#> Error in attachNamespace("atom4R"): namespace is already attached

^{Created on 2022-04-08 by the reprex package (v2.0.1)}

Loading additional packages which depend on atom4R will trigger multiple calls to attachNamespace, which will always trigger that error. Please solve this issue internally, to enable this package to be properly used by other packages.

[eblondel]

it's intended to be used as dependency, but definitely not having to follow your exigences in term of design and timeline, that's really not a way to contribute, and not really in the (good) spirit of ropensci community i've seen so far

[eblondel]

To complement, since your package is actually not using atom4R ie for its primary goal of dealing with the Atom XML encoding/decoding and underlying clients, but just to list the DC elements (that you could have done by parsing DC website vocabularies), I will provide you a util function that can be invoked to only list the DC terms in R.

[mpadge]

Thanks Emmanuel, but deposits does use atom4R to encode, store, validate, and decode elements - the full workflow envisioned here. atom4R is used to store and transmit the entire metadata scheme of deposits objects.

[eblondel]

Then I'll let you know when a viable solution is in place. If what you say is right, i will have issues with geoflow which is supposed to go soon in CRAN in which case the issue will have to be tackled soon. So far the approach I followed in atom4R for listing classes was based on similar way done in other packages I developed in R6 model, packages that are used as dependencies of other packages, and published in CRAN, so I don't really understand your statement when you say that no packages will be able to use atom4R as dependency.

[mpadge]

I don't really understand your statement when you say that no packages will be able to use atom4R as dependency

The code pasted at the outset, and again just above, clearly shows a bug when trying to use this package as a dependency. Just because other packages may already have used it as a dependency and not encountered this bug does not make it go away. Similarly, having

packages that are used as dependencies of other packages, and published in CRAN

does not say make them bug free. The only way for other packages to ensure that this package can be used as a dependency is to use attachNamespace, which is not a real solution for reasons indicated above. Therefore: No packages can or should use atom4R as a dependency in its current state.

I am nevertheless glad that you open to finding a solution here, and look forward to having it implemented. Thanks!

To focus on the issue here, and to reiterate once again: A fix will require the following code to work without having the atom4R namespace attached - it can and should be loaded, but not attached, and therefore not in the search path.

dcentry <- atom4R::DCEntry$new()
#> Loading Atom XML schemas...
#> Loading DCMI vocabularies...
dcentry$setId("my-dc-entry")
dcentry$addDCDate(Sys.time())

[eblondel]

Without having to use 'attachNamespace', just make sure to import properly packages (or some of there functions if you don't to import everything) in your NAMESPACE file (including atom4R in that case), and things will be done IMHO, not sure why you are not doing that if you declare importing them through your DESCRIPTION file. If your intention was to have strong dep on atom4R R6 model, you should import it.

[eblondel]

Fixed with #11 with following integration test cases:

• Integration with zen4R: Add mapping from published ZenodoRecord to atom4R DCEntry zen4R#73
• Integration with cloud4R

[mpadge]

Great fix - thanks!