Skip to content
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

chore: update *requirements.txt #256

Merged
merged 4 commits into from
Apr 23, 2021
Merged

chore: update *requirements.txt #256

merged 4 commits into from
Apr 23, 2021

Conversation

edkerk
Copy link
Member

@edkerk edkerk commented Apr 10, 2021

Main improvements in this PR:

  • chore:
    • update dependencies in *requirements.txt (note format change since pip-tools 5.5.0+)

I hereby confirm that I have:

  • Tested my code with all requirements for running the model
  • Selected devel as a target branch (top left drop-down menu)
  • If needed, asked first in the Gitter chat room about this PR

- note that format of requirements.txt changed since pip-tools 5.5.0+
@edkerk edkerk requested a review from mihai-sysbio April 10, 2021 19:12
Copy link
Member

@mihai-sysbio mihai-sysbio left a comment

Choose a reason for hiding this comment

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

Neat!

@mihai-sysbio
Copy link
Member

It seems a part of the validation fails likely because of memote requiring travis-encrypt which, in turn, requires pyyaml which clashes with ruamel.yaml like this https://stackoverflow.com/questions/9286637/pyyaml-error-attribute-error-no-attribute-load .
Should we keep this part of the validation if we know it will continue to fail for some time?

@mihai-sysbio mihai-sysbio mentioned this pull request Apr 11, 2021
@edkerk
Copy link
Member Author

edkerk commented Apr 11, 2021

@mihai-sysbio Skipping that step can indeed be sensible, is there a memote issue to track whether this is solved?

Meanwhile, would it not work to change line 38 of .github/workflows/yaml-validation.yml from:

        pip install -r requirements/ci-requirements.txt

to

        pip install cobra

?
As memote would probably run in another GH, where the ci-requirements.txt could still be used? Or if it should work in the same GH, then first check for valid YAML after pip install cobra, then run pip install -r requirements/ci-requirements.txt for running memote? All as a workaround, until this issue is solved at memote.

@mihai-sysbio
Copy link
Member

Great idea @edkerk ! I'm not sure why it doesn't work though - from what I can see in the requirements cobra is relying on a single yaml library. Furthermore, when trying this locally I get the same error.

@mihai-sysbio mihai-sysbio force-pushed the fix/requirements branch 2 times, most recently from 95f896b to debde8c Compare April 11, 2021 21:01
@mihai-sysbio
Copy link
Member

@edkerk it looks like there is no problem with the dependencies after all, just unexpected indentation in the yml file. I've manually fixed that for now in bb81286 and the validation passed.

- eccodes and rxnConfidenceScores are not under 'annotation'
- empty references and subsystems should not be written
- generated with RAVEN after commit 99a4536b94b07968a7bbebfb39922c76be7f4e1a
@edkerk
Copy link
Member Author

edkerk commented Apr 13, 2021

A few tweaks in the yml, now directly generated with RAVEN after commit SysBioChalmers/RAVEN@99a4536

@mihai-sysbio
Copy link
Member

A few tweaks in the yml, now directly generated with RAVEN after commit SysBioChalmers/RAVEN@99a4536

nice! imho this PR is ready to be merged

@edkerk edkerk merged commit 28e98f4 into devel Apr 23, 2021
@edkerk edkerk deleted the fix/requirements branch April 23, 2021 18:24
This was referenced Jul 1, 2021
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