Compare grRules as logic in diff_models (order-insensitive) - #74
Merged
Conversation
MATLAB RAVEN's diffModels compares grRules as logic (DNF + sort, so "a and b" == "b and a"), while raven-toolbox's diff_models._normalise_gpr only lowercases and collapses whitespace. Record the AST-based comparison as a pending back-port into raven-toolbox.
Implements the back-port this branch previously only documented. diff_models' GPR check DNF-expands each rule via the existing manipulation.gpr_to_dnf, sorts genes within each isozyme clause and sorts the clauses, so operand order no longer registers as a difference: "a and b" == "b and a", "a or b" == "b or a". The previous _normalise_gpr only lowercased and collapsed whitespace, flagging logically identical rules that differed only in operand order. Uses the reaction's already-parsed cobra .gpr (no re-parse) and reports the original rule strings in the diff message while comparing canonically. A rule cobra cannot parse falls back to the old string comparison, so malformed rules are still compared rather than silently equated to empty. Brings diff_models in line with MATLAB RAVEN's diffModels (RAVEN #686); removes the now-resolved pending-back-port note.
edkerk
changed the title
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
1 participant
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.
diff_models' GPR check compared grRules as text —_normalise_gpronly lowercased and collapsed whitespace — so it flagged two logically identical rules that differed only in operand order (a and bvsb and a) as a difference. MATLAB RAVEN'sdiffModels(RAVEN #686, itself a port of thisdiff.py) already compares them as logic. This brings raven-toolbox into line.Change
_canonical_gprDNF-expands each rule (via the existingmanipulation.gpr_to_dnf), sorts the genes within each isozyme clause and sorts the clauses, so:a and b==b and a(AND operand order)a or b==b or a(isozyme order)(A or B) and C==C and (b or a)(nested, reordered, case-insensitive)a and a==a, and duplicate clauses collapse...while a real logic change (
a and bvsa or b) still registers.It reads the reaction's already-parsed cobra
.gpr(no re-parse) and reports the original rule strings in the diff message while comparing canonically. A rule cobra cannot parse falls back to the old whitespace/lowercase comparison, so malformed rules are still compared rather than silently equated to empty.Tests
tests/test_comparison_diff.pygains order-insensitivity cases (AND/OR reorder, nesting, dedupe), real-difference cases (AND vs OR, different gene), and two end-to-enddiff_modelschecks: operand-order-only models compare equal, an AND→OR change is flagged.pytestfor comparison + expand: 55 passed; ruff, mypy, and the-Wdocs build clean.Replaces this branch's earlier doc-only note with the implementation, and drops the now-resolved pending-back-port entry.