Cycles met ids

[hgscott]

• fix Generalize Metabolite IDs in test_detect_energy_generating_cycles #755
• description of feature/fix- see Generalize Metabolite IDs in test_detect_energy_generating_cycles #755
• tests added/passed
• add an entry to the next release

[Midnighter]

Thank you for the changes! I'm curious to see all the problems this test will have once it is actually run on all the models where it was skipped before.

Please just make the changes for catching exceptions, the rest looks good!

[Midnighter]

The tests fail because I need to make a fix to update the depinfo dependency... Not for you to worry about.

[hgscott]

Thank you for the changes! I'm curious to see all the problems this test will have once it is actually run on all the models where it was skipped before.

Please just make the changes for catching exceptions, the rest looks good!

One thing to note that I found, because the detect_energy_generating_cycles functions returns all reactions where the flux is greater than 0.0, the list can be frighteningly long, but the vast majority of the reactions have a very low flux. So increasing the lower limit from 0.0 to something like 0.0001 decreases the number of reactions found by a lot (and probably gives a more helpful image of what is actually problematic).

I just don't want people to generate the reports and freak out that all their models are broken!

[Midnighter]

That's a good point. I suppose the flux should at least be greater than the tolerance of the solver.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5150a3a) 74.85% compared to head (18fbffe) 74.85%.

❗ Current head 18fbffe differs from pull request most recent head 3e505ca. Consider uploading reports for the commit 3e505ca to get more accurate results

Additional details and impacted files

@@           Coverage Diff            @@
##           develop     #756   +/-   ##
========================================
  Coverage    74.85%   74.85%           
========================================
  Files           50       50           
  Lines         2955     2955           
  Branches       669      669           
========================================
  Hits          2212     2212           
  Misses         649      649           
  Partials        94       94           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Midnighter]

One thing to note that I found, because the detect_energy_generating_cycles functions returns all reactions where the flux is greater than 0.0, the list can be frighteningly long, but the vast majority of the reactions have a very low flux. So increasing the lower limit from 0.0 to something like 0.0001 decreases the number of reactions found by a lot (and probably gives a more helpful image of what is actually problematic).

Now I merged your work without taking this into account. Would you be up for creating another PR to address this?