-
Notifications
You must be signed in to change notification settings - Fork 3
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 credit class schemas to LinkML model #28
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@S4mmyb just opening a draft so we can use comments to chat discuss further changes to this
schema/src/BT01/BT01CreditClass.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@S4mmyb right now this is the BT01 credit class file you had started, except I have modified the ranges to be valid and work with linkml. The issue was we had ranges like schema:Duration
and URL
, but ranges need to be proper data types that linkml understands OR custom data classes.
I'm still not sure what our best option is for specifying a ISO 8601 Duration datatype. I wonder if this might be an easy contribution to LinkML core data types (they have other 8601 date/time types). Or, for now I have specified this in the description eg "Crediting term duration for the project. An ISO 8601 duration.".
schema/src/BT01/BT01CreditClass.yaml
Outdated
SDGs: | ||
description: List of relevant Sustainable Development Goals for this impact. | ||
range: SDG | ||
multivalued: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@S4mmyb currently SDGs
as a slot name doesn't fit our linkml linting configuration (slots should be lower camel case). What about just SDG
?
In the past I have tried to avoid using plural wording for technical properties and just stick with singular words everywhere, knowing that some singular properties might indeed have mutliple values, thinking that the schema will specify if they are multi-valued, not the naming itself. Curious what you think about this more generally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made this change to rename the slots to (lowercase) sdg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Things to pull out of BT01 Credit Class into a common credit class:
- Methodology/Methodology list (added for all credit classes)
- SDG
Used on both credit classes and projects
- Primary impact
- Project benefit
I'll start with these changes and start creating the new structure, then we can continue iterating off of that..
ee7e543
to
84ea3da
Compare
schema/src/BT01/BT01CreditClass.yaml
Outdated
MethodologyList: | ||
class_uri: rfs:MethodologyList | ||
description: List of methodologies for credit generation. | ||
attributes: | ||
itemListElement: | ||
range: Methodology | ||
multivalued: true | ||
description: "List of methodology items." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this intermediary class. I see this was added to try and model https://schema.org/ItemList
Instead, I think we can just reference Methodology
with multivalued: true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I've just rebased on previously merged PRs, these changes should be simpler now.
Unfortunately moving the BT01 Project + Credit class into a src/BT01
subdirectory is breaking the linkml lint and thus builds are failing gen-doc
still works locally. It seems like a bug with the linkml lint command, so for now I'll just remove the BT01 subdirectory. Here is the error: https://github.com/regen-network/regen-data-standards/actions/runs/13501803621/job/37721823950#step:5:59
schema/src/CreditClassInfo.yaml
Outdated
classes: | ||
CreditClassInfo: | ||
class_uri: rfs:CreditClassInfo | ||
abstract: true | ||
title: Credit Class info | ||
description: Base class with common fields used to define credit class info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I learned that there are additional common-metadata that we can use on all parts of the data model. The title
is nice because we can give a more human-readable string that is rendered on top of markdown pages (Credit Class info
instead of CreditClassInfo
)
abstract
is also useful to demonstrate that this is an abstract base class and should not be instantiated/used directly.
schema/src/CreditClassInfo.yaml
Outdated
slot_uri: schema:name | ||
required: true | ||
description: Name of the credit class. | ||
rank: 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rank is another common metadata we can use to set the order of slots
schema/src/CreditClassInfo.yaml
Outdated
- SDG | ||
- ProjectBenefit | ||
- PrimaryImpact |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created these classes in separate yaml files because these classes might be used outside of credit classes. In each of these files I also defined slots that can be used to reference these classes. It's possible that a credit class might have multiple slots referencing the same class for different semantic purposes, in which case each class can define these specific slots themselves.
schema/src/PrimaryImpact.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of "Primary Impact" would it make sense to rename this to a generic "Project Impact' class? That way projects, credit classes, etc could reference multiple impacts, not only a single primary impact ?
schema/src/PrimaryImpact.yaml
Outdated
attributes: | ||
id: | ||
identifier: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of these classes have id
attributes, I believe from @S4mmyb initial migration trying to match the existing schemas. I added the identifier
property so that linkml treats it as such. But I'm curious if we actually want to have full identifiers impacts, benefits, etc? If we are using identifiers does that mean we actually should be pulling from a taxonomy and have a semantic enum?
I need to review existing data more, just leaving the comment while its on my mind cc @clevinson
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it necessarily means that we need a semantic enum, but it should be something that is a full URI.
From the LinkML docs:
the range of an identifier can be any type, but it is a good idea to have these be of type Uriorcurie
A class must not have more than one identifier (asserted or derived). identifier marks the primary identifier.
I'm fine with us using app.regen.network/credit-class/C01 , or http://mainnet.regen.network:1317/regen/ecocredit/v1/class/C01
for these, or some other resource list that we can work on making fully resolvable later (e.g. registry.regen.network/C01 for any arbitrary resource that exists on-chain)
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
schema/src/PrimaryImpact.yaml
Outdated
attributes: | ||
id: | ||
identifier: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it necessarily means that we need a semantic enum, but it should be something that is a full URI.
From the LinkML docs:
the range of an identifier can be any type, but it is a good idea to have these be of type Uriorcurie
A class must not have more than one identifier (asserted or derived). identifier marks the primary identifier.
I'm fine with us using app.regen.network/credit-class/C01 , or http://mainnet.regen.network:1317/regen/ecocredit/v1/class/C01
for these, or some other resource list that we can work on making fully resolvable later (e.g. registry.regen.network/C01 for any arbitrary resource that exists on-chain)
… credit class schemas
Added these in 07ccd43 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notes from a live review with @S4mmyb today.
schema/src/CreditClassInfo.yaml
Outdated
description: Eligible activities for registered projects. | ||
multivalued: true | ||
todos: | ||
- What's the relationship between creditProtocol and approvedMethodologies, in terms of which is required and not? Should one be folded into the other, or atleast align on a single class for the fields' ranges? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current descriptions are good enough. We would like to update the slot names to be better aligned with RDF style predicate names.
Follows from this PR:
|
WIP PR to add schemas for BT01 credit class + project + batches in
schemas/src/BT01
directoryCloses #14