Makes most S4 classes in the package pseudo-namespaced

[hlapp]

In response to already encountered and possible future clashes of S4 class names defined here and elsewhere (see #169), this prefixes most of the classes defined here with "nexml:". The only classes left unchanged (for now?) are the ListOfxxxx classes, which have only minor roles as glorified lists, and the "nexml" root class.

Arguably most if not all clients shouldn't be interacting directly with classes other than the "nexml" class, and hence all others should arguably be considered internal to the package. Hence, how nice and short their names are shouldn't matter to anyone outside the package, whereas short and attractive names ("tree", "meta", "char", "row", "seq", others anyone?) do provide for a heightened risk of clashing with classes defined elsewhere.

Originally, the class hierarchy seems to have been designed with the desire to have a 1:1 correspondence with the NeXML schema. I would argue that makes the mistake of overloading and conflating the concern about mapping properly to and from NeXML schema elements with properly naming S4 classes. This change set solves this by introducing (also pseudo-namespaced, for easy removal in data extractions) slots xmlName and idrefName. For the most part these have the same value for now, but there is no need for them to do so now (unlike previously, where this was all tied to the S4 class name).

Obviously, this change required changes to the test suite across the board because the test suite does a lot of type testing. I tried not to change anything else (for now).

In particular, this maintains the behavior and result returned by get_metadata(), although it would be very straightforward now (and eliminate some code) to automatically consolidate the LiteralMeta and ResourceMeta columns into one and the same column (such as Meta), making metadata considerably less snowflake-like in how they are dealt with. Hence this would make most of #199 superfluous (though not all – the consolidation of property and rel would remain, which one could argue is easy enough to do to simply be left in the hands of a client).

This fixes #169, and in fact also fixes #183, not just in part but all failures reported there.

[cboettig]

Nice, an ambitious update probably well-warranted. Will break backwards compatability with any script calling the base constructors, but probably not likely to be a major issue.

Looks like Travis is upset about the "S4 vignette" not also being patched ...

[hlapp]

I have a fix for the S4 vignette, that's the easy part.

The other problem is the simmap vignette, which is much harder, for several reasons. Specifically:

• The simmap vignette loads a nexml object from a saved data file. I don't know why that was done (loading the original NeXML file surely took a lot less than all the taxize() calls), but the problem is it's of course in the old class names and hence now doesn't work anymore. I can't find the corresponding NeXML file from which it was generated, nor can I find a good candidate for the sets of commands that created it, and that I could thus run again. (Weren't we talking about reproducible science in the RNeXML paper?) I managed to perhaps kludge my way around this by going back to before the class prefixing, loading the file there and saving it out to NeXML (after unloading devtools and a couple other packages brought in by devtools that depend on cli), then reading it back in on this branch.

• The above workaround prompts an error in the simmap code, investigating of which revealed deeper problems. In particular:

  • The simmap_ex data uses LiteralMeta throughout, even though these are nested, and hence the container metas should be really be ResourceMeta. This matters because the code handling metadata is expecting data to be typed correctly. The coercion code to LiteralMeta now treats nested XML as an XML literal, whereas only the coercion to ResourceMeta will expect nested meta elements as children.
  • The culprit looks to be bugs and poor programming choices in the meta() constructor function. As for the latter, calls to new("meta") (which is now new("nexml:meta") should be greatly disfavored if not generally disallowed. A classname of meta (or nexml:meta prefixed) will generally throw a wrench for code retrieving meta elements through the API functions (as opposed to traversing the nexml object). That is, most of the code in the package is not prepared for meta objects that don't know what kind of meta they are (which is in essence what a nexml:meta instance is), and there is no equivalent for these either in the NeXML schema. So kicking this can down the road will sooner or later land the can in a ditch on the side of the road, i.e., become lost. nexml:meta has a role for coercion from XML (and the coercion methods try hard to instantiate the right object rather than just instantiating nexml:meta), but instantiating it seems generally indicative of an unsolved issue in the code and kicking coming up with the real solution down the road.

• One of the main reasons this has gone unnoticed for so long is the poor testing of the meta element handling. Until the recent PRs, the great majority of tests of metadata tested only that the API functions didn't throw an error, not that what they return is what they are expected to return.

I can take a crack at fixing meta() and simmap, but I feel this is significant enough in its own right that it exceeds the scope of this PR. I'd therefore suggest that I simply disable the simmap vignette code, and then work through being able to re-enable it.

[cboettig]

Yup, these sound like pretty major issues in the simmap implementation which will have to be addressed separately. Deprecating the simmap vignette (and associated simmap functions/tests (?) ) so that we can get a clean check on this PR (while also quarantining simmap) seems reasonable.

[hlapp]

Given that we have chosen to merge the alternative in #205, this is now obsolete and I am going to close the PR.