feat: add yaml validation

[mihai-sysbio]

Main improvements in this PR:

This pull request adds YAML validation to all PRs targeting devel and master by running yamllint and then importing the yml format with Cobrapy.
As of now, there are two issues that should be at some point addressed:

• installation of requirements/requirements.txt fails because of a dependency on pywin32. This is not ideal for cross-platform development, nor for GitHub actions (where the cost of running Windows is 2x the cost of Ubuntu). This has been dealt with by replacing the installation step to install just cobra.
• yamllint fails because of indentation. In order to address this, I would propose fixing the exportForGit function upstream, which would require updating RAVEN/io/writeYaml.m. Alternatively, one can ignore indentation problems. The current solution was to simply ignore the problem by continuing on error.
• the Cobrapy import fails because of parsing errors on line 25018, columns 7 and 27. As above, the workflow was allowed to continue on error.

Continuing on error can be misleading, since it gives the impression that everything is alright, even if checking the output things are not. I'd consider removing these flags and allowing the workflow to fail.

[BenjaSanchez]

@mihai-sysbio thanks for working on this, it's great to finally have some CI in the repo :) I agree that continuing on error is misleading... As the main two tests (yaml lint and cobra reading) are failing, maybe it makes sense to first solve at least one of them before we merge this? I can look into the parsing errors. @hao-chalmers were you able to solve linting issues in human-GEM?

[mihai-sysbio]

I totally understand how merging this PR now would result a bunch of red x from failed workflows. At the same time, the underlying issues are really not related to the tests, but with the exportForGit function from Raven, and this PR doesn't change the model in any way.

[BenjaSanchez]

I opened SysBioChalmers/RAVEN#309 which solves the cobrapy import issue (#238), so at least this test will soon pass. Let's hold on merging this PR until after the new yeast-GEM release (#241), to allow enough time for a new release of RAVEN (that way this test will pass once it gets to master).

[BenjaSanchez]

update: I found SysBioChalmers/Human-GEM#173 which solves linting issues + other issues in the yaml file, so I would hold on merging this PR until RAVEN's writeYaml.m ads those corrections. I don't think it's good to release a version that has failing tests, and the fixes seem to be easy enough to implement.

[mihai-sysbio]

Normally I'd preach loose coupling as much as possible (ie to not couple this PR with an unscheduled release of RAVEN), but yeah it doesn't look great to have failing workflows. On the other hand, the released versions already fail the tests, we're just choosing not to show that but modellers might notice anyway SysBioChalmers/Human-GEM#169.

[BenjaSanchez]

@mihai-sysbio at least the linting seems to be quite straightforward to fix? And then both of the tests here would pass... let's continue the discussion at SysBioChalmers/Human-GEM#173 to see how much work would it entail to adapt writeYaml.m, and once we have a better idea we can decide on this PR. To avoid the coupling you mention we could also switch to use RAVEN's devel if a release is too far in the future (we are already doing that with cobratoolbox).

[mihai-sysbio]

It looks like the yml format is still broken.

[edkerk]

This can be easily fixed by adapting RAVEN's writeYaml to fit the changes from writeHumanYaml?

[mihai-sysbio]

I suspect so, but I'm not sure how the discussion in SysBioChalmers/Human-GEM#173 concluded.

[edkerk]

yeastGEM.yml should now be fixed, available in PR #255

[mihai-sysbio]

After merging in the fix/various branch, I can confirm that the yml is valid.

I've noticed that the import with cobrapy stage fails because it fails to install kiwisolver. Knowing that there are already some dependabot alerts for the requirements, I would propose to solve these as an independent issue.