Fixing a couple of minor bugs and added new variable class

[remidhum]

Bug fixes:

• fixed the apply_directionality function to properly obtain the forward and backward variables wanted.
• fixed the rebuilding of models without an objective function to now have a dummy one.

Enhancement:

• added a new class (ForwardBackwardUseVariable) to easily deal with inactive reactions in a model.

[remidhum]

The test fails because my 'fix' for the apply_directionality is due to the fact I pass the solution.raw directly instead of the solution object itself. I don't know if it is more elegant to deal with this use case or not. Please let me know what you think of it so I can make the appropriate changes to the PR (fix the test or revert the first commit). Cheers :)

[remidhum]

I also notice that the strip_from_integer_variables() function takes advantage of the rebuild_obj_from_dict() function to not set an objective value for the sampling later on. This causes the commits in this PR to result not be able to sample properly.

I now wonder whether the strip_from_integer_variables() should be modified to revert to what is expected of it or if the changes in rebuild_obj_from_dict() should be completely rethought.

[psalvy]

Hi @remidhum Thanks for these useful contributions !

The test fails because my 'fix' for the apply_directionality is due to the fact I pass the solution.raw directly instead of the solution object itself. I don't know if it is more elegant to deal with this use case or not.

I think passing the solution object is more appropriate, as it is more high level. Alternatively you can write a check that verifies the type of the solution passed (DataFrame or solution object) and assigns the proper object to the solution variable. I would actually encourage this as it is more in accordance of what is done in cobra, pyTFA and ETFL.

I also notice that the strip_from_integer_variables() function takes advantage of the rebuild_obj_from_dict() function to not set an objective value for the sampling later on. This causes the commits in this PR to result not be able to sample properly.

I am of the opinion that stripping from integer variable should not edit the objective function, and the user should provide their own ahead of time

I'll let you update your code according to these comments and accept the PR when you're done :)

[codecov[bot]]

Codecov Report

Merging #35 into master will decrease coverage by 0.1%. The diff coverage is 68.18%.

@@            Coverage Diff             @@
##           master      #35      +/-   ##
==========================================
- Coverage   71.06%   70.95%   -0.11%     
==========================================
  Files          46       46              
  Lines        3383     3398      +15     
==========================================
+ Hits         2404     2411       +7     
- Misses        979      987       +8

Impacted Files	Coverage Δ
pytfa/optim/variables.py	78.12% <37.5%> (-1.77%)	:arrow_down:
pytfa/io/dict.py	80.97% <80%> (-0.22%)	:arrow_down:
pytfa/analysis/manipulation.py	65.71% <88.88%> (+1.19%)	:arrow_up:
pytfa/core/model.py	84.23% <0%> (-0.55%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9724094...a6eec48. Read the comment docs.

[remidhum]

Thank you for your suggestion regarding the apply_directionality() function. Hopefully the check is OK with you.

I am of the opinion that stripping from integer variable should not edit the objective function, and the user should provide their own ahead of time

I agree with you. It would seem however that I was wrong regarding the objective function. Quite confusingly, I cannot reproduce the error I was observing the other day...