search

ropensci / EML

Ecological Metadata Language interface for R: synthesis and integration of heterogenous data
https://docs.ropensci.org/EML
Other
97 stars 33 forks source link

make `get_attributes` fully compatible with annotations #336

Closed jeanetteclark closed 2 years ago

jeanetteclark commented 2 years ago

Some names in the attribute list were re-named to fit with other tools, but not the ones associated with URIs. I just added a few lines to fix those so that all of the annotation information is retrieved.

cboettig commented 2 years ago

LGTM.

@amoeba can you give this a quick :eyes: and lemme know if we are good to merge?

amoeba commented 2 years ago

This looks totally fine, though I wasn't able to reproduce the behavior I think, @jeanetteclark, you're aiming to adjust. Are you seeing attribute lists come back with column names like "annotation.valueURI.valueURI" and such?

Running of master, I don't see that:

> library(EML)
> 
> doc <- read_eml("https://arcticdata.io/metacat/d1/mn/v2/object/doi%3A10.18739%2FA25M6280M")
> 
> al <- get_attributes(doc$dataset$dataTable[[1]]$attributeList)
> names(al$attributes)
 [1] "id"                  "attributeName"       "attributeLabel"      "attributeDefinition"
 [5] "unit"                "numberType"          "propertyLabel"       "propertyURI"        
 [9] "valueLabel"          "valueURI"            "measurementScale"    "domain"
jeanetteclark commented 2 years ago

@amoeba sorry I missed this. We were seeing the annotations get dropped entirely from existing attribute lists that had them, though when I reinstall EML from main I see expected behavior as shown above. So maybe everyone is just on an older version of EML on our server? Not sure I'll need to check

jeanetteclark commented 2 years ago

I had one of our interns test by reinstalling using devtools::install_github("ropensci/EML") and it works as expected, sorry for the false alarm! closing

amoeba commented 2 years ago

Glad to hear that, at least. Thanks for checking @jeanetteclark!

jeanetteclark commented 2 years ago

reopening this, the error has cropped up again twice so something is wrong. here is a pid on arctic if you want to test @amoeba: urn:uuid:bcee3610-6c78-4590-90c3-8d21d4d2ea88

unfortunately it is not public read

amoeba commented 2 years ago

Thanks @jeanetteclark, I'll have a look this week or next.

amoeba commented 2 years ago

Hey @jeanetteclark, sorry to take my time getting to this.

As far as I can tell, this still affects the CRAN release of EML but not the latest code from git. I'm fairly certain 26c8dd7c069baf6ce85ea2ff78c344df3c68d0e6 fixes this issue but I know that code isn't on CRAN. Yet.

That said, I think your patch improves get_attributes because all four annotation-related columns get handled explicitly and uniformly, rather than only having code to handle propertyURI and valueURI.

@cboettig this is good to merge and can we plan to get a CRAN release out as soon as practical? Happy to help there.

jeanetteclark commented 2 years ago

Thanks @amoeba - I'd also be willing to help get the CRAN release in to whatever degree I can. Right now I think our team are working in a bit of version stew (some off my fork, some off the tip of master here, some on the CRAN version) and a CRAN release would really help I think