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 space_group_symmetry_operations_xyz and sub-definition for sym ops #4

Merged

Conversation

ml-evs
Copy link

@ml-evs ml-evs commented Dec 23, 2023

Currently uses the proposed ECMA regexp at Materials-Consortia#488

Schemas will fail to generate unless we add pattern Materials-Consortia#460 (comment) in Materials-Consortia#460

@ml-evs
Copy link
Author

ml-evs commented Jan 4, 2024

This is currently blocked by Materials-Consortia#486

@rartino
Copy link
Owner

rartino commented Jan 5, 2024

@ml-evs So Materials-Consortia#486 is merged, so that part is fine.

I know things have gotten a bit messy with the two PRs, and perhaps I should just merge Materials-Consortia#460 into this one and close Materials-Consortia#460, now that the two are in sync. But, in any case, this PR (as in Materials-Consortia#445) needs the meta-schema updated for the pattern field. But, ok, I'll merge this, and can add that.

@rartino
Copy link
Owner

rartino commented Jan 5, 2024

@ml-evs But, eh, wait, how completely does this PR cover Materials-Consortia#464? I thought we at least modified the definition of some of the other symmetry fields, but maybe we didn't(?)

@rartino rartino merged commit 1ade132 into rartino:add-property-definitons-3 Jan 5, 2024
@ml-evs
Copy link
Author

ml-evs commented Jan 5, 2024

Hi @rartino, indeed this needed to be updated with the results of that PR (and also to include the other fields). I'll try to get around to that soon.

@rartino
Copy link
Owner

rartino commented Jan 6, 2024

@ml-evs The changes relating to the other fields seemed quite minor, so in an effort to get things ready while I have time, I've added/modified them myself. I've also modified the meta schema for property definitions to allow the 'pattern' field for strings (but when/if Materials-Consortia#490 is merged, we should adjust the description). However, I though it to be fair to leave it to you add the fancy regex you worked out. Note: the place to add it is now in: schemas/src/defs/v1.2/properties/optimade/common/space_group_symmetry_operation_xyz.yaml which is the definition of a single symop, which is inherited by space_group_symmetry_operations_xyz.yaml.

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.

2 participants