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

bug identified in development branch #628

Closed
feiranl opened this issue Jun 12, 2023 · 7 comments
Closed

bug identified in development branch #628

feiranl opened this issue Jun 12, 2023 · 7 comments
Assignees
Labels

Comments

@feiranl
Copy link
Collaborator

feiranl commented Jun 12, 2023

import and export cycle gives an error:

model = importYaml('Human-GEM.yml');
>> exportYaml(model, 'Human-GEM.yml');
Index exceeds the number of array elements. Index must not exceed 13087.

Error in exportYaml>writeField (line 190)
            list = field{pos};  %subSystems already comes in a cell array

Error in exportYaml (line 86)
    writeField(model, fid, 'subSystems',           'txt', i, '  - subsystem')

This is due to the subsystems length is different with other rxn related field.
Screenshot 2023-06-12 at 10 51 09 AM

@feiranl feiranl added the bug label Jun 12, 2023
@feiranl feiranl self-assigned this Jun 12, 2023
@mihai-sysbio
Copy link
Member

Oh that's surprising to see. There is a GH workflow that runs the yaml conversion check. It looks like when the test fails the workflow still passes - have a look at the step Run conversion script for this workflow ron on the merged PR. Looking back through the results, the problem might have been introduced in #606.

@mihai-sysbio
Copy link
Member

My suggestion is that the fix should also include a change in the test such that when it fails the action also fails. After this, the fix of the yaml file should be committed, and we should see the workflow pass.

@feiranl feiranl removed their assignment Jun 12, 2023
@feiranl
Copy link
Collaborator Author

feiranl commented Jun 12, 2023

it errors in the check, but didnot fail in the action:

{�Index exceeds the number of array elements (13087).

Error in exportYaml>writeField (line 190)
list = field{pos}; %subSystems already comes in a cell array

Error in exportYaml (line 86)
writeField(model, fid, 'subSystems', 'txt', i, ' - subsystem')

Error in testYamlConversion (line 25)
exportYaml(model,'testYamlConversion.yml');

Error in run (line 91)
evalin('caller', strcat(script, ';'));
}�

@haowang-bioinfo maybe you can fix this?

@haowang-bioinfo haowang-bioinfo self-assigned this Jun 12, 2023
@haowang-bioinfo
Copy link
Member

haowang-bioinfo commented Jun 12, 2023

Oh that's surprising to see. There is a GH workflow that runs the yaml conversion check. It looks like when the test fails the workflow still passes - have a look at the step Run conversion script for this workflow ron on the merged PR.

yes indeed, I observed similar cases before, i.e. testYamlConversion failed locally but the action still passed on GitHub

@haowang-bioinfo maybe you can fix this?

yes, working on this

@haowang-bioinfo
Copy link
Member

haowang-bioinfo commented Jun 12, 2023

Looking back through the results, the problem might have been introduced in #606.

the error appears to be introduced in #619, instead of #606. Because testYamlConversion would pass by using importYaml before 619

this error is neither from #606 nor #619, really not sure where was it introduced. A plausible reason might be various external dependencies that have been changing all the time

@haowang-bioinfo
Copy link
Member

haowang-bioinfo commented Jun 12, 2023

@feiranl the error encountered during yaml import and export cycle is likely fixed by commit 182010d in #629

@mihai-sysbio
Copy link
Member

Based on the merged pull requests I will close this issue, please re-open if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants