fix: require cobra>=0.31.1 to avoid deepcopy RecursionError - #35
Merged
Conversation
copy.deepcopy(ec_model) (and of any cobra.Model) raised RecursionError on Python 3.14. The cause was in cobra, not geckopy: Reaction.__copy__/ __deepcopy__ delegated to super() to avoid recursing into themselves, relying on copy memoizing an object before recursing into its state -- Python 3.14's copy module no longer does so for that code path. Every reaction holds its metabolites and every metabolite holds the reactions it participates in, so the resulting cycle exists in any real model, full ecModels included. Fixed upstream in cobra 0.31.0 by deleting those overrides. Bump the floor to 0.31.1 and add a regression test that deep-copies a model with a reaction<->metabolite cycle.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
python3 -m pytest -qwas failing 146/1330 tests withRecursionError: maximum recursion depth exceededfrom insidecopy.deepcopy's_reconstruct, across many unrelated test files.The root cause is not in geckopy's
EcModel. A bare two-objectcobra.Model(oneReaction, oneMetabolite, no geckopy code involved) reproduces the sameRecursionErroroncopy.deepcopy. The actual cause iscobra.Reaction.__copy__/__deepcopy__: they delegate tosuper()as a trick to avoid infinitely recursing into themselves, which relies oncopymemoizing the in-progress object before recursing into its state. Python 3.14'scopymodule no longer does that for this code path, so the reaction↔metabolite back-reference that exists in any real cobra model (every reaction holds its metabolites; every metabolite holds the reactions it participates in) sendsdeepcopyinto genuine infinite recursion. This is why the failures spanned reaction- and kcat-related tests across the whole suite rather than being confined toEcModel.copy().This was already fixed upstream: cobra 0.31.0 ("revise the copy mechanisms") deletes the overriding
__copy__/__deepcopy__methods onReactionentirely, letting the defaultcopy/__getstate__/__setstate__machinery (which memoizes correctly) handle it. Verified against cobra 0.32.1.Changes
cobradependency floor from>=0.29to>=0.31.1inpyproject.toml, with a comment explaining why.test_deepcopy_does_not_recurse_on_reaction_metabolite_cycleintests/test_ec_model.py) that deep-copies anEcModelwith a reaction↔metabolite cycle and checks the copy is independent and correctly rebound. Confirmed it reproduces theRecursionErrorwhen the pre-0.31.1Reaction.__copy__/__deepcopy__overrides are reinstated.CHANGELOG.mdentry underUnreleased / Fixed.Test plan
python3 -m pytest -q --timeout=180— 1329 passed, 1 deselected, 1 xfailed (was 146 failed before, with cobra <0.31.1)tests/test_ec_model.py— 25 passed, including the two new deepcopy testsRecursionErrorwhen cobra's pre-0.31.1Reaction.__copy__/__deepcopy__are reinstated, and passes with them removed (i.e. cobra>=0.31.1)