Skip to content

fix-the-autoannotate-metadata-workflow - #421

Merged
comcon1 merged 9 commits into
NMRLipids:mainfrom
mdondrup:test_annot_wf
Jun 21, 2026
Merged

fix-the-autoannotate-metadata-workflow #421
comcon1 merged 9 commits into
NMRLipids:mainfrom
mdondrup:test_annot_wf

Conversation

@mdondrup

@mdondrup mdondrup commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread Molecules/membrane/BOGUS/metadata.yaml Outdated
@mdondrup
mdondrup requested review from comcon1 and korbinib June 9, 2026 18:35
mdondrup and others added 3 commits June 10, 2026 09:06
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@comcon1 comcon1 closed this Jun 10, 2026
@comcon1 comcon1 reopened this Jun 10, 2026
@mdondrup

Copy link
Copy Markdown
Collaborator Author

Hi @comcon1 and @korbinib. Now the workflow should be working and is prepared to use the CAS API token.

@comcon1

comcon1 commented Jun 13, 2026

Copy link
Copy Markdown
Member

Hi @comcon1 and @korbinib. Now the workflow should be working and is prepared to use the CAS API token.

Could you please explain this PR? Is it a PR for testing WF or it's a PR that we are going to merge in the end? Does it depend on the PR that is opened in the FAIRMD_lipids? Should we wait until we merge that one first?

@mdondrup mdondrup changed the title testing wf fix-the-autoannotate-metadata-workflow Jun 13, 2026
@mdondrup

Copy link
Copy Markdown
Collaborator Author

This PR was initially meant to find why the automatic metadata annotation workflow was failing. Now the wf itself is working. However it depends on NMRLipids/FAIRMD_lipids#506 to generate syntactically correct metadata files. The CAS_API_KEY is already injected for use with the python script. It does no harm as it is only used by the new version of autocomplete_metadata.py.

@comcon1 comcon1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

developer's script has been merged

Comment on lines 26 to 27
git clone --depth 1 https://github.com/NMRlipids/FAIRMD_lipids.git "$RUNNER_TEMP/Databank"
pip install "$RUNNER_TEMP/Databank"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some non-blocking comments. You may think about it if you want. I don't have strong opinions here.

  1. I think we could install latest stable version using pip install fairmd.lipids.

  2. If we need a script from develop, we could also checkout repo from stable tag

  3. I don't like very much that we are using out-of-package scripts in another repository; I think it's a bit wrong responsibility.. but I don't see an easy suggestion on how to improve it.

@comcon1
comcon1 merged commit 8bd0250 into NMRLipids:main Jun 21, 2026
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