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 a Dataset Profile example (CO2 dataset) #84

Merged
merged 25 commits into from
Aug 14, 2024

Conversation

bact
Copy link
Contributor

@bact bact commented Jun 4, 2024

Inspired from https://github.com/owid/co2-data/

Structurally and semantically validated against all tools here: https://github.com/spdx/spdx-3-model/blob/main/serialization/json_ld/validation.md

TODOs:

@bact bact marked this pull request as draft June 4, 2024 06:55
@bact bact marked this pull request as ready for review June 5, 2024 16:00
Signed-off-by: Arthit Suriyawongkul <[email protected]>
@bact bact changed the title [WIP] Add a Dataset Profile example (CO2 dataset) Add a Dataset Profile example (CO2 dataset) Jun 11, 2024
bact added 5 commits June 14, 2024 18:25
Signed-off-by: Arthit Suriyawongkul <[email protected]>
Signed-off-by: Arthit Suriyawongkul <[email protected]>
Signed-off-by: Arthit Suriyawongkul <[email protected]>
Use non-UUID IDs for diagram for readability
(long IDs made the diagram got trimmed due to its size)

Signed-off-by: Arthit Suriyawongkul <[email protected]>
Signed-off-by: Arthit Suriyawongkul <[email protected]>
@bact
Copy link
Contributor Author

bact commented Jun 16, 2024

Experiment notes:

  • Successfully Validated with all the tools (ajv, check-jsonschema, pyshacl) listed at https://github.com/spdx/spdx-3-model/blob/main/serialization/json_ld/validation.md
  • Warning messages of ajv and check-jsonschema sometimes are not very helpful
    • If there're warnings/errors, trying to remove some objects from your JSON. Once the smaller JSON got validated, gradually add few more.
    • spdx3ToGraph can be more handy to detect errors in the first runs as it can provide more useful error messages (use this Add debug message for validate() maxhbr/spdx3ToGraph#2 to get more exact location of error)
    • But spdx3ToGraph validation will not check the cardinality, you still have to use ajv or check-jsonschema for that. (The tool is meant primarily for visualization btw)
      • If maxCount is *, the data type must be an array
  • A lot of errors found in this try (and in few other examples) is about serialized names. So if TODO in Where should we display a serialized name of a property/class in the documentation? spdx-spec#975 is completed, it will help a lot.
  • A PlantUML diagram, generated from spdx3ToGraph can be useful to understand the overall structure
    • However, due to limitation of PlantUML visualizer (I use ones from PlantUML.com, online and offline), if you have a very long spdxId (based on UUIDv4, for example), your diagram are very likely to be overflowed/got cropped.
    • For this example, I edited the generated PlantUML file to have shorter spdxIds before I submit it again to the visualizer. Just to have a diagram that actually fit. (The IDs in JSON-LD file are untouched)
  • A real-time validation in an editor would help. VS Code supports JSON validation with a schema. If you familiar with VS Code, please help review this PR to add VS Code validation to the validation document Add VS Code validation and common errors spdx-3-model#790

bact added 2 commits June 17, 2024 00:52
- Remove availableFrom relationship, as it duplicates with the originatedBy
- Shorten IDs for the diagram

Signed-off-by: Arthit Suriyawongkul <[email protected]>
@bact
Copy link
Contributor Author

bact commented Jun 19, 2024

This this how I put AnyLicneseInfo in this example, to please the SHACL validator -- as a workaround for the lacking of ListedLicense at this moment. (This will be removed once spdx/LicenseListPublisher#183 is implemented).

LicenseExpression is a subclass of AnyLicenseInfo and is valid to be used a to in "has license" relationships.
The spdxId is set to be identical to an expected license IRI. This means when the license (CC-BY-4.0) is available as ListedLicense (and use this IRI), this LicenseExpression workaround element can be removed without any need to make change in "has license" relationships.

    {
      "type": "simplelicensing_LicenseExpression",
      "spdxId": "https://spdx.org/licenses/CC-BY-4.0",
      "creationInfo": "_:creationinfo",
      "simplelicensing_licenseExpression": "CC-BY-4.0",
      "simplelicensing_licenseListVersion": "3.24.0"
    }

--

For this example, we can decide which version of BOM we would like to have:

  1. a BOM that is valid as of actual ontology (current ontology without ListedLicense)
  2. a BOM that is valid as of ontology as designed (future ontology with ListedLicense)

There can be 3 decision options:

a. If (1) is ok, we can merge as it is. And once spdx/LicenseListPublisher#183 is implemented, we can revise the BOM again to remove the workaround LicenseExpression.

b. If (2) is preferred, I can remove the workaround LicenseExpression element now, so it can get merge (after other necessary revisions).

c. Last option is doing nothing until we have all the required ListedLicense and then go with (2).

@bact
Copy link
Contributor Author

bact commented Jun 19, 2024

@rgopikrishnan91 this PR please Gopi

bact added 2 commits June 30, 2024 23:53
Signed-off-by: Arthit Suriyawongkul <[email protected]>
Signed-off-by: Arthit Suriyawongkul <[email protected]>
@bact
Copy link
Contributor Author

bact commented Aug 1, 2024

@rgopikrishnan91 @bennetkl please kindly review. Thank you.

Copy link

@bennetkl bennetkl left a comment

Choose a reason for hiding this comment

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

Looks good, approved.

@bennetkl
Copy link

bennetkl commented Aug 1, 2024

@kestewart I believe you will have to merge, I don't have permission for this one.

@kestewart
Copy link
Contributor

DIscussed in AI call. Merging.

@kestewart kestewart merged commit 39f65de into spdx:master Aug 14, 2024
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