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

Feature 109 funding #163

Merged
merged 8 commits into from
May 14, 2021
Merged

Feature 109 funding #163

merged 8 commits into from
May 14, 2021

Conversation

mbjones
Copy link
Collaborator

@mbjones mbjones commented May 6, 2021

These are the proposed changes for Funding to use the @reverse keyword on fundedItem. It includes both updates to the Dataset guidance docs, a new example, a new figure, and an ADR describing the impact.

Discussion of these features was in issue #109

@mbjones mbjones added Update Documentation updates to the guidance docs proposed decision labels May 6, 2021
@mbjones mbjones added this to the v1.3 milestone May 6, 2021
@mbjones mbjones self-assigned this May 6, 2021
@mbjones mbjones requested a review from ashepherd May 6, 2021 00:40
nokome and others added 2 commits May 7, 2021 11:39
Fix some typos and consistent application of back ticks to terms.
@mbjones mbjones requested a review from datadavev May 10, 2021 23:52
@mbjones
Copy link
Collaborator Author

mbjones commented May 10, 2021

@datadavev can you give this a quick review so I can merge it into develop if there is nothing controversial? I think you already looked over the changes based on your comments in issue #109 . Thanks.

Copy link
Collaborator

@datadavev datadavev left a comment

Choose a reason for hiding this comment

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

Overall this seems great to me as a mechanism to describe funding sources of a dataset. There are a couple minor typos, and a suggestion to add a reference to the spec for @reverse. Otherwise this is good to go.

The only issues I have are in the broader context of assuming that there's some special characteristic of the JSON structure of the JSON-LD block that confers meaning, when in fact, there is none. The logical representation of the content using @reverse or @graph with appropriate targets is the same, and the RDF representation of each is identical. Hence, my impression is that emphasis should be more on the correct representation of the relations between entities rather than promoting one particular construct (i.e. @reverse) when there is no logical difference.

decisions/109-funding.md Outdated Show resolved Hide resolved
guides/Dataset.md Outdated Show resolved Hide resolved
guides/Dataset.md Outdated Show resolved Hide resolved
@mbjones
Copy link
Collaborator Author

mbjones commented May 11, 2021

Yeah, makes sense. How about if I add a sentence at the end explaining that and alternative would be to use the @graph representation?

@datadavev
Copy link
Collaborator

Yeah, makes sense. How about if I add a sentence at the end explaining that and alternative would be to use the @graph representation?

Kind of feels more like there should be a general guideline on JSON-LD, but adding a sentence here could be helpful. Perhaps something like:

Note that using @reverse is a convenience that can simplify the construction of JSON-LD for describing datasets since it places the funders as objects within the SO:Dataset object rather than composing multiple objects within a @graph list which may be more complicated to generate. Logically there is no difference. In either case, consumers of JSON-LD should take appropriate steps to process the content to achieve an expected JSON structure (e.g. with framing) or treat the JSON-LD as an RDF graph and query accordingly.

@mbjones
Copy link
Collaborator Author

mbjones commented May 11, 2021

Thanks @datadavev I incorporated your feedback into the branch and PR.

@mbjones mbjones linked an issue May 12, 2021 that may be closed by this pull request
Copy link
Contributor

@smrgeoinfo smrgeoinfo left a comment

Choose a reason for hiding this comment

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

In the first paragraph on Funding, the Crossref Funder ID is (I think...) referred to with three different names.

...institutional identifier such as a ROR, or ISNI identifier, or a more detailed Crossref Funder ID. The ROR for the National Scien....ifiers as well. The Funder ID has the advan.... When possible, providing both a ROR and FunderId is helpful. ...

It would be much clearer if the same name was used, maybe 'Funder ID' and link to Crossref (https://www.crossref.org/services/funder-registry/) the first time its used?

@mbjones
Copy link
Collaborator Author

mbjones commented May 12, 2021

Thanks, @smrgeoinfo, good catch. I just harmonized the language used to "Funder ID" and added a link in sha e87b2e0. Does that address your feedback?

@mbjones mbjones merged commit e87b2e0 into develop May 14, 2021
@mbjones mbjones added accepted decision Issues on which a decision was accepted for release. and removed proposed decision labels May 14, 2021
@smrgeoinfo
Copy link
Contributor

looks good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted decision Issues on which a decision was accepted for release. Update Documentation updates to the guidance docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adopt funding property if it gets accepted
4 participants