Emit curated input/output stoichiometry; fix .env loading and duplicate reaction rows - #49
Merged
Conversation
…up rows - Curated stoichiometry: _resolve_vr_entities hardcoded every reaction input/output edge to stoichiometry=1, discarding the curated coefficient. Now reads Reactome's coefficient from the input/output relationship (new get_reaction_io_stoichiometry, same rel.stoichiometry property that get_complex_components uses) and attaches it per node — each expanded set member inherits its entity's coefficient; coefficients sum when two annotated entities map to one node; defaults to 1 absent curation. This feeds the stoichiometry_weighted export/pathway-rollup view (previously always 1). - .env loading: create-pathways.py used dotenv_values (a local dict that never populates os.environ), so NEO4J_URL/USER/PASSWORD/OUTPUT_DIR/LOG_LEVEL from .env were silently ignored and the connector fell back to hardcoded defaults. Switched to load_dotenv (non-clobbering) so credentials can be supplied when the Neo4j instance needs them. - get_reaction_connections: added RETURN DISTINCT — the hasEvent* traversal returns the same reaction pair via multiple sub-pathway paths, inflating the cached reaction_connections.csv. Behaviour-preserving (consumers dedupe). Test updated to assert curated coefficients flow through; full non-Neo4j suite passes (922). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Three fixes from a bug sweep. The headline is curated stoichiometry finally reaching the emitted networks.
Changes
logic_network_generator.py,neo4j_connector.py) —_resolve_vr_entitieshardcoded every reaction input/output edge tostoichiometry=1, discarding the curated coefficient. It now reads Reactome's coefficient from theinput/outputrelationship (newget_reaction_io_stoichiometry, samerel.stoichiometrypropertyget_complex_componentsalready uses) and attaches it per node — each expanded EntitySet member inherits its entity's coefficient, coefficients sum when two entities map to one node, and it defaults to 1 absent curation. Regenerated networks carry real coefficients (2/3/4/6/12/14 on assembly subunits). Phase-3 emission already consumed these maps, so no downstream change was needed..envloading (create-pathways.py) — useddotenv_values(a local dict that never populatesos.environ), soNEO4J_*/OUTPUT_DIR/LOG_LEVELfrom.envwere silently ignored and the connector fell back to hardcoded defaults. Switched toload_dotenv(non-clobbering) so credentials can be supplied when needed.get_reaction_connectionsDISTINCT (neo4j_connector.py) — thehasEvent*traversal returns the same reaction pair via multiple sub-pathway paths, inflating the cachedreaction_connections.csv. Behavior-preserving (consumers already dedupe).Scope note
The curated stoichiometry currently feeds the
stoichiometry_weightedexport/pathway-rollup view (what the pathway-browser overlay shows). Wiring it into the deltasignal solver was built and A/B-tested separately and A/B'd as a small regression, so it stays off by default — see #48 for the full result. This PR is the generator-side correctness fix, independent of that.Verification
Test updated to assert curated coefficients flow through; full non-Neo4j suite passes (922). The 8 pre-existing failures require a live Reactome Neo4j and are unaffected by this change.
🤖 Generated with Claude Code