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

Constructors not speaking EML 2.2.0 #287

Open
amoeba opened this issue Nov 15, 2019 · 6 comments
Open

Constructors not speaking EML 2.2.0 #287

amoeba opened this issue Nov 15, 2019 · 6 comments

Comments

@amoeba
Copy link
Collaborator

amoeba commented Nov 15, 2019

Stevan Earl noted that the constructor functions don't seem to have arguments for the EML 2.2.0 element.

As an example, if you try eml$project(), you won't see an award argument.

It looks like the args are just the EML 2.1.1 args. I checked the 2.2.0 database/JSON file and it looks good so it seems like the bug might somewhere like https://github.com/ropensci/EML/blob/master/data-raw/construct_complexTypes.R.

I do see this:

options("emld_db" = "eml-2.2.0") ## hmm, not working?
options("emld_db" = "eml-2.1.1")
so I wonder if this ever worked. @jeanetteclark do you know?

@mbjones
Copy link
Member

mbjones commented Nov 15, 2019

Maybe just the second options call on line 7 needs to be commented out so that 2.2.0 is set for the eml_db option?

@cboettig
Copy link
Member

Yup, Matt's suggestion sounds good, though I'm not sure these constructors were ever quite as robust or convenient as you might want them to be.

I have an experimental project over at https://github.com/cboettig/build.eml which tries to generate constructor functions for all complex types which are actually first class R package functions, with documentation etc. That project could use some more love but I think the basics should work at least as well as the built-in templates? Ultimately that does feel like a better way to go.

(Of course this is also by scraping the XSD files, something that usually pushes me past the limits of my poor grasp of XPATH).

(emld also has a lightweight, relatively undocumented template() method for creating templates -- I think it should be up-to-date 2.2.0 but haven't checked that either!)

@jeanetteclark
Copy link
Contributor

I can't get construct_complexTypes.R to run with the EML version set to 2.2.0.

Here is the traceback:

eml <- lapply(who, EML:::template_constructor)
Error in parse(text = f) : <text>:1:63: unexpected ':'
1: function(section = list(), para = list(), markdown = NULL, xml:
                                                                  ^ 
  4. parse(text = f) at template_constructor.R#12
  3. eval(parse(text = f)) at template_constructor.R#12
  2. FUN(X[[i]], ...) 
  1. lapply(who, EML:::template_constructor) 

which leads me to think it is due to thexml:lang elements here (not present in eml-2.1.1, not sure why). Unfortunately I really am not sure how to fix this in a good way but it seems like it needs to be handled in emld::template() maybe?

@jeanetteclark
Copy link
Contributor

I figured out a solution that just tosses the xml:lang out but I'm not sure if that is what we want or not... See branches below:

https://github.com/jeanetteclark/EML/tree/constructor_functions
https://github.com/jeanetteclark/emld/tree/constructor_fix

What I'm even less sure of is how we can support both the EML 2.2.0 and EML 2.1.1 versions of the constructor functions. Maybe an EML::eml_version() helper that will call emld::eml_version and run construct_complexTypes.R? Or we keep two rda files in the package and load one of them according to the options set by emld? Nothing very satisfying is jumping out at the moment

@cboettig
Copy link
Member

@jeanetteclark thanks for this.

I think the xml:lang element in the eml-2.2.0.json files should be tossed out (probably should be removed from the .json file directly, that file just exists to assist with the constructors in the first place.

Yeah, we could probably cludge something together to let a user call eml_version and have it switch out which rda file it loads for the constructors, but it having constructors that behave differently as a side effect of another function call seems like it could lead to more trouble.

Still, I think breaking out the constructor task into a separate package like I've tried to do in build.eml might be a better way to go. Then you get first-class function constructors, like so: https://cboettig.github.io/build.eml/reference/eml.html

With better schema parsing, these docs could indicate better what is required vs optional, what is repeatable, and add some decent examples, but these first class functions are already better than the constructors tacked in here (in EML package) I think?

I suppose one could have two versions of the package, one for 2.2.0 and one for 2.1.1 if necessary.

amoeba added a commit to amoeba/emld that referenced this issue Jul 15, 2020
Partially addresses ropensci/EML#287 by changing emld_db to built off of the 2.2.0 JSON file. Removing the xml:lang attributes from the 2.2.0 JSON file is required because of how the templating function breaks when an argument with a colon is run through parse(). the xml:lang attribute can still be set on any EML element (list) manually.

HT @jeanetteclark for a first pass impl of this: ropensci/EML#287 (comment).
amoeba added a commit to amoeba/EML that referenced this issue Jul 15, 2020
Addresses ropensci#287. Note that dynamically switching the constructors based on version isn't supported here: We just have constructors for a single version (2.2.0) at a time.

Note this depends on ropensci/emld#66 landing in master to work right because we build the constructors from the copy of the JSON file in master.
@amoeba
Copy link
Collaborator Author

amoeba commented Jul 15, 2020

Just sent in two (dependent) PRs for EML (#308) and emld (ropensci/emld#66) to fix the xml:lang parsing issue and set up constructors for EML 2.2.0 by default. There's more thinking to do here (or in another issue) but this at least gets things working. I think we might be able to sneak this into the next release if others are keen.

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

4 participants