Skip to content

add NSF award helper#149

Merged
jeanetteclark merged 13 commits intomasterfrom
nsf_award_helper
Nov 20, 2019
Merged

add NSF award helper#149
jeanetteclark merged 13 commits intomasterfrom
nsf_award_helper

Conversation

@jeanetteclark
Copy link
Collaborator

This function generates an EML project section from a list of NSF award numbers

I'm VERY excited about it 🚀

@jeanetteclark
Copy link
Collaborator Author

last chance to review @dmullen17 I'd like to merge this soon

Copy link
Member

@dmullen17 dmullen17 left a comment

Choose a reason for hiding this comment

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

Looks great to me! I tested all of the edge cases I could think of but wasn't able to make it fail ungracefully. Added a couple general syntax comments but none of those changes are necessary.

result <- lapply(award_nums, function(x){
url <- paste0("https://api.nsf.gov/services/v1/awards.json?id=", x ,"&printFields=coPDPI,pdPIName,title")

t <- jsonlite::fromJSON(url)
Copy link
Member

Choose a reason for hiding this comment

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

Any reason this is named t? Doesn't need to be changed just wondering

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not really, just a short placeholder name. response would be a natural name but the return value for fromJSON already has response in it

R/eml.R Outdated

# create function to extract first name and middle initial (if present)
# as firstName, and whatever else exists as lastName
extract_name <- function(x){
Copy link
Member

Choose a reason for hiding this comment

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

should this be defined outside the function body as a helper?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hm. I don't think so. This is pretty specific to how the NSF API formats the names and I can't imagine it would be that useful anywhere else. Maybe it isn't good practice to define a function within another function though?

Copy link
Member

Choose a reason for hiding this comment

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

That was my thought but it doesn't really matter. No need to move it.

R/eml.R Outdated
proj <- list(title = titles, personnel = p_list, award = awards)
}


Copy link
Member

Choose a reason for hiding this comment

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

Do you want to explicitly return(proj) here? I expected its behavior to print the project section to the console when I didn't assign it to a variable.

@jeanetteclark
Copy link
Collaborator Author

Thanks for the suggestions @dmullen17! merging in now

@jeanetteclark jeanetteclark merged commit a7b4ef4 into master Nov 20, 2019
laijasmine pushed a commit that referenced this pull request Oct 2, 2020
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