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

Add metadata #421

Merged
merged 5 commits into from
Jan 3, 2017
Merged

Add metadata #421

merged 5 commits into from
Jan 3, 2017

Conversation

lindsayplatt
Copy link

Now each gsplot object will have a top-level list called metadata. This PR adds 2 fields: created which is the date the object is/was created and gsplot.version the gsplot version used to create the object. Both can be manually input as arguments to gsplot().

This PR also adds to access functions to get those metadata values - whatDate and whatVersion.

See #304

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 82.226% when pulling d0d1e7f on lindsaycarr:addMetadata into e839398 on USGS-R:master.

Copy link
Contributor

@jiwalker-usgs jiwalker-usgs left a comment

Choose a reason for hiding this comment

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

I like this, I'm wondering if there is other metadata we should consider. Also, are there other accessor methods that could follow the what pattern here?

gsplot.default <- function(...,config.file=NA, theme=NA, frame.plot=TRUE) {
object <- gsplot(list(global=list('config'=list(frame.plot=frame.plot,
gsplot.default <- function(..., created=Sys.Date(),
gsplot.version=packageDescription("gsplot", fields = "Version"),
Copy link
Contributor

Choose a reason for hiding this comment

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

might use packageVersion("gsplot") here instead

Copy link
Member

Choose a reason for hiding this comment

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

better yet is

packageVersion(getPackageName())

so we don't need to modify it if we change the name of the package.

Copy link
Author

Choose a reason for hiding this comment

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

I tried packageVersion("gsplot") at first, but it returns a list instead of a character string. packageDescription("gsplot", fields = "Version") returns a single string. To get the output of packageVersion into a single string seemed more complex than using packageDescription.

I will make the package reference generic per @jread-usgs's suggestion.

@ldecicco-USGS
Copy link
Member

What is this magic "Approved" that you guys are doing?

Copy link
Member

@ldecicco-USGS ldecicco-USGS left a comment

Choose a reason for hiding this comment

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

Ahhh! Cool.

@lindsayplatt
Copy link
Author

@jiwalker-usgs I think there were other metadata that people suggested, but we decided that #304 would just be the gsplot version and date.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 82.226% when pulling 2327280 on lindsaycarr:addMetadata into e839398 on USGS-R:master.

@jordansread jordansread merged commit 98e3e0a into USGS-R:master Jan 3, 2017
@lindsayplatt lindsayplatt deleted the addMetadata branch January 3, 2017 20:36
@lindsayplatt
Copy link
Author

@jread-usgs the appveyor build failed...is that OK?

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.

5 participants