Add extra schema checker jobs to CI #577
Conversation
ml-evs
added
blocking-release
ml-evs
force-pushed
the
ml-evs/add-schema-checker
branch
2 times, most recently
from
0baf57e to
a2cb422
Compare
|
This PR now has a checker for:
|
merkys
left a comment
merkys
left a comment
There was a problem hiding this comment.
Thanks for developing this @ml-evs! This PR adds the much-needed automatic validation of the specification. However, I believe, there are certain cases when it might run into false-positives and false-negatives. Moreover, the code is quite tightly coupled with v1.2 and v1.3, thus it would need updating for each subsequent versions.
I think the optimade-property-tools might be reused for such tasks as entry type/property lookup inside the property definitions. If not, I have recently written some code for the COD implementation and could reuse it to expose simple executables for such tasks.
In any case, I am OK with merging the PR as it is, as it would help us in developing the v1.3.0 specification and schemas. However, I would keep a mental note to revisit the code later.
| check_property() { | ||
| local entry_type=$1 | ||
| local property=$2 | ||
| local version=${3:-v1.2} # Default to v1.2 |
There was a problem hiding this comment.
Instead of hard-coding, maybe we can extract version numbers (here and below for v13_path) from optimade.rst or CHANGELOG.md? Otherwise we may forget bumping them.
| local version=${3:-v1.2} # Default to v1.2 | ||
|
|
||
| # Check in the main entry type directory first | ||
| local primary_path="schemas/src/defs/$version/properties/optimade/$entry_type/$property.yaml" |
There was a problem hiding this comment.
I think the proper way to check for the existence of a property is:
- Take the entry types and their URLs from
schemas/src/defs/$version/standards/optimade.yaml - Consult the entry type URLs (
schemas/src/defs/$DEFINITION_VERSION/entrytypes/optimade/$ENTRY_TYPE.yaml) for properties
Important point here is that $version and $DEFINITION_VERSION might not be the same (see schemas/src/defs/v1.3/standards/optimade.yaml for example). I have recently implemented this for COD, maybe I could backport the code here. But maybe we want to do something really simple like find schemas/src/defs/*/$property.yaml instead?
| else | ||
| # Check if it exists in any entry type | ||
| found=false | ||
| for et in structures files references calculations; do |
There was a problem hiding this comment.
This again is hard-coded, will need updating for every new entry type.
There was a problem hiding this comment.
Indeed, but its chicken-and-egg whether we add it here or wait for a folder name to be made with the relevant schemas. At least we could move it to the top of the file so its clear.
There was a problem hiding this comment.
I've looked again and this is unnecessarily hardcoded, as it's already extracted from the spec further up. Might be a bit trickier to extract versions but I'll fix this at least.
ml-evs
force-pushed
the
ml-evs/add-schema-checker
branch
from
0c905db to
ad21fe3
Compare
Add examples for site_coordinate_span in YAML schema
…symmetry_operations_xyz
rartino
removed
the
blocking-release
This commit adds scripts to verify that all properties defined in the specification (optimade.rst) have corresponding schema definition files. - extract_entry_fields.sh: Extracts field names from the Entry List section of optimade.rst, looking for properties with ~~~~ underlines - check_property_definitions.sh: Compares extracted fields against schema definitions in schemas/src/defs/, reporting missing definitions and orphaned schema files - Updated Makelocal-audit to include audit_property_definitions target in the main audit suite The checker validates that: 1. All properties with ~~~~ formatting have corresponding YAML schema files 2. Common properties exist in the core/ directory 3. Entry-specific properties exist in the appropriate optimade/<entry_type>/ directories 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The scripts now continue checking all properties and report all errors at the end, rather than exiting on the first error.
ml-evs
force-pushed
the
ml-evs/add-schema-checker
branch
from
5560ee1 to
e8b3f8b
Compare
|
After the meeting we decided to remove the |
Splitting off uncontroversial |
ml-evs
changed the title
ml-evs
added
the
status/delay
Closes #575
I've also tweaked some of the rst roles in the spec to maybe allow us to scrape the spec for all fields (in a2cb422). It might be worth adding some special role to the headers that define fields to make this even easier, otherwise I'll concoct some grep incantation to try and do the same job without needing any changes to the spec.