Skip to content
This repository has been archived by the owner on Jan 8, 2025. It is now read-only.

Add header to model #321

Merged
merged 5 commits into from
Aug 31, 2023
Merged

Add header to model #321

merged 5 commits into from
Aug 31, 2023

Conversation

brandomr
Copy link
Collaborator

This PR is in support of DARPA-ASKEM/Model-Representations#56 which requires that various fields be nested under header in the AMR.

The goal of this PR is to flexibly support this new request while not forcing breakage of all existing AMRs by adding a helper utility to tds/modules/model/utils.py called restructure_model_header which checks to see if the header exists and if not, performs the requisite nesting.

Until this is merged, all TA1 produced AMRs will be rejected by TDS since they now have the header.

"model_version": "1.0",
},
"username": "user123",
"timestamp": "2023-08-30T00:00:00", # Example datetime value in ISO format
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we are missing the "model": {}, line here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't actually return the model aspect for the description endpoints

model = restructure_model_header(model)

# Rename 'schema' to 'model_schema' if present
if "model_schema" in model["header"]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This check could probably be folded in to the restructure_model_header() call if that makes sense. I'll put a code suggestion in a different comment.

tds/modules/model/utils.py Outdated Show resolved Hide resolved
tds/modules/model/utils.py Outdated Show resolved Hide resolved
@brandomr brandomr merged commit b9d23e1 into main Aug 31, 2023
@brandomr brandomr deleted the feat/amr-header branch August 31, 2023 14:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants