LCSB-BioCore / COBREXA.jl

Constraint-Based Reconstruction and EXascale Analysis
http://bit.ly/COBREXA
Apache License 2.0
42 stars 8 forks source link

Solve convert(StandardModel, model::JSONModel) issue #822

Closed josePereiro closed 4 months ago

josePereiro commented 5 months ago

Description

Hi, I'm trying to load a json model (download link: elife-36842-supp10-v2.json) from this paper.

The following code throw an error

using COBREXA
model = COBREXA.load_json_model(fn);
convert(StandardModel, model)

The error is related with the management of missing keys in the JSONModel... Mustly notes and annotations...

This pull request aim to solve them by returning default (empty) values when need it and generalizing the parsing mechanism.

Saludos

Checklist

Feel free to open PRs that do not satisfy the whole checklist. On the other hand, all these are required for merging.

stelmo commented 5 months ago

Hi there! We are actually in the process of releasing COBREXA v2 (see here). We can definitely take a look at this PR, but you might want to consider porting it over here. We split out the model loading interface from the analysis functions, and introduced a much cleaner framework to make building complex models really easy.

exaexa commented 5 months ago

Hi @josePereiro, thanks a lot! I guess the patch looks OK, I'll try to merge this here (and take it apart into the new version's repositories) ASAP.

Among other things the (external) CI is currently a little dead; I'll check the stuff manually later.

For the "formatting" CI, if you click details there should be a patch to apply, but you don't need to care there (I'll do).

josePereiro commented 5 months ago

Hi, it's good to hear about the progress on COBREXA v2. It will be great to have functionality separated into modules. I would suggest placing a notice in the COBREXA v1 package about this "news"; otherwise, it may seem like the project's development/maintenance status is lower.

I've opened a new PR on JSONFBCModels. Most of the default functionality was already implemented, but the model at hand has deeper nested annotations, so the parser fails...

exaexa commented 4 months ago

/format

github-actions[bot] commented 4 months ago

:x: Auto-formatting triggered by this comment failed, perhaps someone pushed to the PR in the meantime?

exaexa commented 4 months ago

I tested this manually, all is OK. Thanks!

exaexa commented 4 months ago

@josePereiro I will try to release this asap (not today, too late). If I forget about it PLEASE do feel free to remind me, I want to release this but in case it's not out next wednesday or so I just forgot. :D