-
Notifications
You must be signed in to change notification settings - Fork 0
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
Implement migrator that creates/populates workflow_chain_set
(file: migrator_from_X_to_PR9.py
)
#127
Conversation
workflow_chain_set
collection
Here are the commands I ran: ``` # Install black. pip install "black>=23.1,<25" # Run black. black nmdc_schema/migrators/migrator_from_X_to_PR9.py ```
Here's the command I'm using to run the doctests in this migrator: poetry run python -m doctest -v nmdc_schema/migrators/migrator_from_X_to_PR9.py |
I'll focus on these two issues:
|
The cautionary message was: ``` No adapter was specified. Migration capability will be limited. ``` Note: That message still appears with other doctests that aren't part of this file.
The log from the most recent GitHub Actions failure says:
The installation of a Python package failed. The failure is not related to the doctests. I opened a ticket in |
Katherine has a PR up to solve this, so its being worked on. |
workflow_chain_set
collectionworkflow_chain_set
collection (file: migrator_from_X_to_PR9.py
)
workflow_chain_set
collection (file: migrator_from_X_to_PR9.py
)workflow_chain_set
(file: migrator_from_X_to_PR9.py
)
Implementation of this migrator is currently blocked by the Minter (or, rather, "a Minter") not allowing people to mint IDs for |
@eecavanna @turbomam this migrator is finally ready for a final review and can be merged into main afterwards. The workflochain ids have been pre-minted. The change with the project.makefile was because I edited it and then recently change it back to match main - so it should match main. Not sure why its showing those interesting changes. |
I'm reviewing this now. I will make some changes to variable names and push a new commit today. |
As @brynnz22 said, the lines that GitHub is showing as being added to the |
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.
Looks good to me! I am comfortable with this being merged into the main
branch. Once it's merged in, it'll be easier for me to test "all Berkeley migrators" in series.
Hi @turbomam, once you merge this branch into |
Migrator creates the
workflow_chain_set
and maps allWorkflowExecution
subclasses to a workflow chain instance by the omics processing id in thewas_informed_by
slot.OmicsProcessing
toDataGeneration
as it refers to theomics_processing_set
.Address changes in PR #9