Skip to content

Conversation

@brookslogan
Copy link
Collaborator

WIP on updating data attribution. We still lack Authors@R entries and/or inst/COPYRIGHTS, and maybe some individual @source / other roxygen needs updated.

@brookslogan brookslogan changed the title Change to just file LICENSE, add @source for CTIS data Update data attributions Jun 14, 2023
@brookslogan brookslogan force-pushed the lcb/adjust-attribution branch from 78ed183 to 01b71e3 Compare June 14, 2023 17:46
DESCRIPTION Outdated
person("Johns Hopkins University Center for Systems Science and Engineering", role = "dtc", comment = "Owner of COVID-19 cases and deaths data sourced from the COVID-19 Data Repository"),
person("Johns Hopkins University", role = "cph", comment = "Copyright holder of COVID-19 cases and deaths data sourced from the COVID-19 Data Repository"),
person("Carnegie Mellon University Delphi Group", role = "dtc", comment = "Owner of masking and social-distancing data sourced from the COVID-19 Trends and Impacts Survey"),
person("Carnegie Mellon University", role = "cph", comment = "Copyright holder of masking and social-distancing data sourced from the COVID-19 Trends and Impacts Survey")
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: Not sure this is true. Is Delphi the copyright holder?

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 quite an answer. I based this on https://cmu-delphi.github.io/delphi-epidata/api/covidcast_licensing.html:

[...]

You may use this data for any purpose, provided you attribute us as the original source, as shown above.

So if this is imprecise, we also need to update there.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Dropping the CMU CTIS cph line. We can resolve it as part of issue cmu-delphi/epiprocess#348

@nmdefries
Copy link
Collaborator

@brookslogan

We still lack Authors@R

Right now, we only list Dan and data citations. Who else is an author? Is it only you?

@nmdefries
Copy link
Collaborator

nmdefries commented May 31, 2024

What did you want to include in inst/COPYRIGHTS? It seems that the recommendation is "If the copyright and authorship of a package is particularly complex, you can use plain-text files inst/COPYRIGHTS and inst/AUTHORS to provide more information." At the moment, we're including copyright holders of data as authors (based on how some other packages do it), so I'm not sure that's necessary for us.

Base automatically changed from lcb/activate-roxygen-markdown to main September 3, 2024 22:16
@nmdefries nmdefries marked this pull request as ready for review September 4, 2024 16:11
Copy link
Collaborator Author

@brookslogan brookslogan left a comment

Choose a reason for hiding this comment

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

I can't approve this as it's "my" PR. But I think it's good to go after addressing the inline comments. I like the current format but think there's some missing attribution + licensing stuff for doctor visits and Canada data. I've also made a whole bunch of minor mechanical suggestions (probably should have just made a commit, sorry) + a typo fix.

@nmdefries
Copy link
Collaborator

@brookslogan I think this is actually ready now.

Copy link
Collaborator Author

@brookslogan brookslogan left a comment

Choose a reason for hiding this comment

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

Great! I think this can be merged now after:

  • updating for epiprocess update (grad_employ_subset needs slight tweak)
  • a few typo fixes.

That said, there does seem to be some extra files / complications / package size that could be avoided, though these don't seem to be impacting the functionality.

[I would approve this but I can't, as it's "my" PR.]

@nmdefries nmdefries force-pushed the lcb/adjust-attribution branch from 5313fa6 to c6f8fcc Compare October 1, 2024 21:52
@nmdefries nmdefries self-requested a review October 1, 2024 21:54
Copy link
Collaborator

@nmdefries nmdefries left a comment

Choose a reason for hiding this comment

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

Rubberstamping, as I did most of the development. See Logan's review above.

@nmdefries nmdefries merged commit d91aaee into main Oct 1, 2024
2 checks passed
@nmdefries nmdefries deleted the lcb/adjust-attribution branch October 1, 2024 22:08
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