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

Added functions to add Arctic Report Card annotations into data packages #203

Merged
merged 21 commits into from
Jul 9, 2024

Conversation

justinkadi
Copy link
Contributor

  • Added 2 functions into ontology.R for creating new annotations for the Arctic Report Card datasets

eml_arcrc_key_variable_annotation(valueLabel), eml_arcrc_essay_annotation(valueLabel)

property: "isAbout", value: "key variable" (e.g. "isAbout" "sea surface temperature")
property: "influenced", value: "essay topic" (e.g. "influenced" "Greenland Ice Sheet Indicator")

  • Added 1 function into eml.R for actually adding the new annotation into the EML doc

eml_arcrc_add_annotation(doc, property, label)
eml_arcrc_add_annotation(doc, "isAbout", "sea surface temperature")
eml_arcrc_add_annotation(doc, "isAbout", c("sea ice", "sea surface temperature"))
eml_arcrc_add_annotation(doc, "influenced", "Greenland Ice Sheet Indicator")

  • Added test files and documentation into test_ontology.R and test_eml.R

@justinkadi justinkadi requested a review from jeanetteclark July 3, 2024 19:46
Copy link
Collaborator

@jeanetteclark jeanetteclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code looks good. could you have a look at the check results here: https://github.com/NCEAS/arcticdatautils/actions/runs/9783945702/job/27013717921?pr=203 we should get those tests passing again (even though they are not from your changes), and try to resolve the notes as well. I think the note regarding exported objects (:::) can be ignored, but we should try to fix the global variable ones. IIRC I think that those result from issues with dplyr syntax

@mbjones
Copy link
Member

mbjones commented Jul 3, 2024

I think the note regarding exported objects (:::) can be ignored

There is almost never a good reason to use :::, and it will never pass muster in an externally published package on CRAN. Why do you need it here? If the method you need is not exported, then that should be changed, rather than going around it. By not exporting a method, the package author is explicitly saying "never use this method, it is private to the package". Failing to heed that will lead to broken code eventually. See: https://stat.ethz.ch/R-manual/R-devel/library/base/html/ns-dblcolon.html

@jeanetteclark
Copy link
Collaborator

not sure why the ::: got put there in the first place - they are calls to methods from the dataone package, I think put in when Bryce first wrote the package

@jeanetteclark jeanetteclark merged commit 62c4df8 into NCEAS:main Jul 9, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

3 participants