core: add the API surface MSCommunity needs from ModelSEEDpy - #158
Open
freiburgermsu wants to merge 1 commit into
Open
core: add the API surface MSCommunity needs from ModelSEEDpy#158freiburgermsu wants to merge 1 commit into
freiburgermsu wants to merge 1 commit into
Conversation
MSCommunity (https://github.com/ModelSEED/MSCommunity) currently cannot run against released ModelSEEDpy. It imports six names that exist only in the freiburgermsu fork, and calls sixteen MSModelUtil/FBAHelper members that main does not define, so `import mscommunity` raises on the first line of commhelper.py against every published version. That blocks distributing MSCommunity on PyPI, since PyPI does not accept direct-URL dependencies. This adds exactly what MSCommunity needs and nothing else. Every change is additive; no existing signature, behaviour or export is altered. core/exceptions.py ParameterError, ObjectAlreadyDefinedError, NoFluxError, ObjectiveError, ModelError -- five classes, in the style of the four already here. core/fbahelper.py FBAHelper.isnumber, .mediaName, .rxn_mets_list, .solution_to_variables_dict, .convert_kbase_media, plus module-level bioFlux_check and minimizeFlux_withGrowth. The fork's minimizeFlux_withGrowth logged through icecream; that call is dropped so this adds no new dependency. core/msmodelutl.py MSModelUtil gains add_medium, add_minimal_objective_cons, add_objective, carbon_exchange_list, carbon_exchange_mets_list, create_constraint, exchange_mets_list, metabolites_set, remove_constraint, remove_cons_vars, run_fba and standard_exchanges. __init__ gains four optional keyword arguments -- copy, environment, climit and o2limit -- all defaulting to the current no-op behaviour, so existing `MSModelUtil(model)` callers are unaffected. The uptake-limit logic is factored into apply_uptake_limits() so it can be called independently. get_pkg_mgr is now passed self.model rather than model, which is identical when copy=False and correct when copy=True. core/__init__.py Re-export bioFlux_check and minimizeFlux_withGrowth. The new exceptions need no change here; `from modelseedpy.core.exceptions import *` already covers them. Verification, on this branch against ModelSEED/ModelSEEDpy main (a5ea486): - ModelSEEDpy's own suite: 39 passed, identical to the unmodified baseline. - MSCommunity's suite (ModelSEED/MSCommunity main, b5f37c4): 72 passed, against 7 collection errors before this change. Formatted with black, matching .pre-commit-config.yaml. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HFX6x9QKfUuHSuAdhmgL12
Member
Author
On the red checks — they are pre-existing on
|
| Date | Workflow | Result | Commit |
|---|---|---|---|
| 2026-07-21 | Run Tox | failure | Testing for conflicts |
| 2026-07-21 | Run Pre-Commit | failure | Testing for conflicts |
| 2026-06-17 | Run Tox | failure | Testing for conflicts |
| 2026-06-17 | Run Pre-Commit | failure | Testing for conflicts |
| 2026-05-08 | Run Tox | failure | Testing for conflicts |
| 2026-05-08 | Run Pre-Commit | failure | Testing for conflicts |
The tox failure is dependency rot rather than anything in the source:
ERROR: Failed to build 'scikit-learn' when getting requirements to build wheel
ModuleNotFoundError: No module named 'pkg_resources'
setup.py:38 pins scikit-learn == 0.24.2, whose build requires pkg_resources; setuptools removed that module in 81.0. So the environment can no longer be created, and the suite never runs. Worth a separate PR — either unpin, or pin setuptools<81 for the build.
Two things I checked so the formatting question does not muddy the review:
black==22.8.0, the version pinned in.pre-commit-config.yaml, reports "4 files would be left unchanged" on the files this PR touches.- Running both suites locally against this branch: ModelSEEDpy
tests/gives 39 passed, byte-identical to the unmodifiedmainbaseline, and MSCommunitytests/gives 72 passed wheremainproduces 7 collection errors.
Happy to rebase once CI is healthy again if you would rather see it green before merging.
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.
Why
MSCommunity cannot run against released ModelSEEDpy. It imports six names that exist only in the
freiburgermsufork, and calls sixteenMSModelUtil/FBAHelpermembers thatmaindoes not define, soimport mscommunityraises on the first line ofcommhelper.pyagainst every published version:That blocks distributing MSCommunity on PyPI, because PyPI does not accept direct-URL/VCS dependencies — there is no way to express "install the fork" in package metadata. This PR closes the gap so MSCommunity can depend on plain
modelseedpy.What
Exactly what MSCommunity needs, nothing else. Every change is additive — no existing signature, behaviour or export is altered.
core/exceptions.py— five classes, in the style of the four already there:ParameterError,ObjectAlreadyDefinedError,NoFluxError,ObjectiveError,ModelError.core/fbahelper.py—FBAHelper.isnumber,.mediaName,.rxn_mets_list,.solution_to_variables_dict,.convert_kbase_media; plus module-levelbioFlux_checkandminimizeFlux_withGrowth. The fork'sminimizeFlux_withGrowthlogged throughicecream; that call is dropped so this adds no new dependency.core/msmodelutl.py—MSModelUtilgainsadd_medium,add_minimal_objective_cons,add_objective,carbon_exchange_list,carbon_exchange_mets_list,create_constraint,exchange_mets_list,metabolites_set,remove_constraint,remove_cons_vars,run_fba,standard_exchanges.__init__gains four optional keyword arguments —copy,environment,climit,o2limit— all defaulting to the current no-op behaviour, so existingMSModelUtil(model)callers are unaffected. The uptake-limit logic is factored intoapply_uptake_limits()so it can be called independently.get_pkg_mgris now passedself.modelrather thanmodel: identical whencopy=False, and correct whencopy=True.core/__init__.py— re-exportbioFlux_checkandminimizeFlux_withGrowth. The new exceptions need no change here;from modelseedpy.core.exceptions import *already covers them.Verification
Run on this branch against
main@a5ea486:tests/tests/(ModelSEED/MSCommunitymain @b5f37c4)Formatted with
black, matching.pre-commit-config.yaml.Notes for review
ModelSEED/ModelSEEDpymain, not from the fork'smain, so the diff is only these four files rather than the fork's accumulated drift.minimizeFlux_withGrowthis imported by MSCommunity but never called. It is included so MSCommunity's import line resolves and because it pairs naturally withbioFlux_check; if you would rather not carry it, MSCommunity can drop the import instead and it can come out of this PR.core/fbahelper.pyrather than reviving the fork'score/msminimalmedia.py, to keep the change small. Say the word if you would prefer that module.🤖 Generated with Claude Code
https://claude.ai/code/session_01HFX6x9QKfUuHSuAdhmgL12