SysBioChalmers/yeast-GEM

fix: cobrapy parsing errors of yaml

Closed this issue · 11 comments

BenjaSanchez commented

mentioned in #236:

the Cobrapy import fails because of parsing errors on line 25018, columns 7 and 27

The line in question is - name: [cytochrome c]-L-lysine

However the error message is a bit criptic:

ParserError: while parsing a block mapping
  in "yeastGEM.yml", line 25018, column 7
expected <block end>, but found '<scalar>'
  in "yeastGEM.yml", line 25018, column 27

I'm also assuming that more parsing errors will come as we solve them, so this will remain open until all are adressed.

Midnighter commented

I wonder if the YAML parser expects a list there. Since it is legal to have square brackets denote lists. How is this line written? It should probably be:

- name: "[cytochrome c]-L-lysine"
BenjaSanchez commented

@Midnighter the yaml file is written using https://github.com/SysBioChalmers/RAVEN/blob/master/io/writeYaml.m and aims to mimic the format of https://github.com/opencobra/cobrapy/blob/devel/cobra/test/data/mini.yml. The name fields in mini.yml don't use "", so is it only when the name starts with brackets that "" should be used? Other cases that have brackets seem to work fine in yeastGEM.yml, e.g. 2-[3-carboxy-3-(methylammonio)propyl]-L-histidine. For simplicity, should maybe all names use ""?

Midnighter commented

Yes, it is because it starts with a bracket. I think it would be safest to put all strings in quotes. However, I don't know if this is easily possible with ruamel.yaml since it might add quotes only when necessary and you'd have conflicts with cobrapy again.

Midnighter commented

Apparently, preserve_quotes=True would be the way to go. Needs a small fix on cobrapy.

  • 👍1
BenjaSanchez commented

great, I will propose a similar flag for the RAVEN function then. Compatibility with cobrapy is for now only needed in one direction (i.e. model being loaded with cobrapy), so there should not be any conflicts?

Midnighter commented

That's easier then, indeed.

BenjaSanchez commented

update: this was solved with #253 and will be closed on the next release

mihai-sysbio commented

Between this and the other fixes in PR #255, I think it should be safe to close this issue.

BenjaSanchez commented

@mihai-sysbio the issue is indeed solved, but only in devel. Therefore it's easier to leave it open until a new release, as the issue is still present in master, and could lead to users opening duplicate issues.

mihai-sysbio commented

Oh I thought that was covered by this line in the issue template:

  • Checked that a similar issue does not exist already
BenjaSanchez commented

@mihai-sysbio the fixed in devel label is intended to make it easier on the user, so that they don't have to look through closed issues as well. Note that the default searching filter in GitHub is is:issue is:open. Perhaps this would be clearer if that line read:

  • Checked that a similar open issue does not exist already
  • 👍1