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

qcschema_input and qcschema_molecule updating #60

Merged
merged 14 commits into from
Feb 25, 2019
Merged

Conversation

loriab
Copy link
Collaborator

@loriab loriab commented Feb 24, 2019

Description

  • From discussion in add schema fields to molecule #59, adds req'd fields {'schema_name': 'qcschema_molecule', 'schema_version': 2} within molecule schema. closes add schema fields to molecule #59 . qcelemental 0.2.7 will understand this as dtype=2.
  • Effectively in the QCA software stack, qc_schema_[in|out]put has been moving to qcschema_[in|out]put, so this PR expresses the patterns in regex. Also removes the possibility to say sldkfjsl_qc_schema, which I've never actually seen encoded. Postfix still allowed.

Status

  • Ready to go

@loriab
Copy link
Collaborator Author

loriab commented Feb 24, 2019

ok, passing. do squash. and if I've completely messed up the versioned testing by editing the test cases, let me know.

@dgasmith
Copy link
Collaborator

Any reason to allow postfix? I think it was always the intent that exact match was required.

@loriab
Copy link
Collaborator Author

loriab commented Feb 25, 2019

postfix is what's allowing qcschema_input and qcschema_output at the moment.

@dgasmith
Copy link
Collaborator

The base_schema is an implementation detail in this repo and is overridden per schema molecule/input/output at the moment (or should be). Might be worth removing this line from the base schema.

@loriab
Copy link
Collaborator Author

loriab commented Feb 25, 2019

so long as neither input/output try to validate against base_schema, I think this'll do.

Copy link
Collaborator

@dgasmith dgasmith left a comment

Choose a reason for hiding this comment

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

LGTM

@dgasmith dgasmith merged commit c855c2f into MolSSI:master Feb 25, 2019
@loriab loriab deleted the patch-7 branch August 12, 2020 22:15
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.

add schema fields to molecule
2 participants