Skip to content

Conversation

@stellaprins
Copy link
Collaborator

To test whether update to github actions works as expected.

@robertvi
Copy link
Collaborator

@stellaprins looks like the "Test test_suite output regeneration" action is now failing because we have deleted the test above it in the yml and therefore pymetadata module is no longer installed when the test is run. To fix this you could reorder the tests so that the new "test omex file creation" test is above the "Test test_suite output regeneration" test, then pymetadata module should be available.

… for GitHub Actions to be performed successfuly
…s needed for GitHub Actions to be performed successfuly" As this was not the problem.

This reverts commit 35d071a.
@stellaprins
Copy link
Collaborator Author

stellaprins commented Jun 13, 2024

The new "test omex file creation" test still gives a Docker error

  • docker.errors.ContainerError: Command '-i /root/in/LEMS_NML2_Ex9_FN_missing_xmlns.omex -o /root/out' in image 'ghcr.io/biosimulators/tellurium' returned non-zero exit status 1

There are two Biosimulators warnings

File "/home/runner/work/SBMLShowcase/SBMLShowcase/SBML/../utils/init.py", line 267, in biosimulators_core
client.containers.run(f"ghcr.io/biosimulators/{engine}",
File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/docker/models/containers.py", line 905, in run
raise ContainerError(
docker.errors.ContainerError: Command '-i /root/in/LEMS_NML2_Ex9_FN_missing_xmlns.omex -o /root/out' in image 'ghcr.io/biosimulators/tellurium' returned non-zero exit status 1: b'/usr/local/lib/python3.9/site-packages/biosimulators_utils/warnings.py:31: BioSimulatorsWarning:\r\n\r\n\x1b[33mCOMBINE/OMEX archive has warnings.\r\n - The manifest does not include its parent COMBINE/OMEX archive. Manifests should include their parent COMBINE/OMEX archives.\x1b0m\r\n\r\n/root/in/LEMS_NML2_Ex9_FN_missing_xmlns.omex is not a valid COMBINE/OMEX archive.\n - The format of the archive must be [http://identifiers.org/combine.specifications/omex, not [http://identifiers.org/combine.specifications:omex.\n](http://identifiers.org/combine.specifications:omex%60./n)'
Error: Process completed with exit code 1.

@robertvi
Copy link
Collaborator

pymetadat 0.4.0 is probably an old version https://github.com/OpenSourceBrain/SBMLShowcase/actions/runs/9498644739/job/26177817122?pr=25#step:8:15 I reported this bug to the owner and he fixed it so the url generated will be correct if you can get pip to install the latest version of pymetadata matthiaskoenig/pymetadata#50

@robertvi
Copy link
Collaborator

i think the thing about the manifest is only a warning and will not cause it to fail by itself

@robertvi
Copy link
Collaborator

pymetadata 0.4.2 requires python >= 3.8 therefore we cannot test omex creation using this package on python 3.8 without supplying our own workaround for the ": instead of / in url" bug
see here

@robertvi
Copy link
Collaborator

maybe just skip this one test for v3.8?

@pgleeson
Copy link
Member

@robertvi yes, certainly skip py3.8 if there is a package that doesn't work with it. It's nearing "end of life" anyway https://devguide.python.org/versions/

@robertvi
Copy link
Collaborator

If we're happy to skip python 3.8 from all the non-omv.yml checks then this should be ready to merge.

@robertvi robertvi marked this pull request as ready for review June 14, 2024 11:09

def create_omex(sedml_file, sbml_file):

def add_xmlns_sbml_attribute_if_missing(sedml_filepath, sbml_filepath):
Copy link
Member

Choose a reason for hiding this comment

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

Is this method a duplication of the method below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, Rob split up the "add_xmlns_sbml_attribute_if_missing" I created into "add_xmlns_sbml_attribute" and "xmlns_sbml_attribute_missing". I think we can delete "add_xmlns_sbml_attribute_if_missing".

@pgleeson pgleeson merged commit a358185 into master Jun 19, 2024
@stellaprins stellaprins deleted the test_omex branch October 9, 2024 13:31
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.

4 participants