Fixes #3421 Enable PK-Sim ExpressionProfile from MoBi.R#3420
Conversation
📝 WalkthroughWalkthroughThis PR introduces expression profile creation functionality by adding error message helpers, converting BuildingBlockCreator to a static class with a new CreateExpressionProfile API, and adding comprehensive test coverage for the new creation patterns and error handling. Changes
Sequence DiagramsequenceDiagram
actor Client
participant BuildingBlockCreator
participant SpeciesRepository
participant ProfileBuilder
participant ExpressionProfile
Client->>BuildingBlockCreator: CreateExpressionProfile(category, molecule, species)
BuildingBlockCreator->>SpeciesRepository: Validate species exists
alt Species Valid
SpeciesRepository-->>BuildingBlockCreator: Species confirmed
BuildingBlockCreator->>ProfileBuilder: Build profile by category type
ProfileBuilder->>ExpressionProfile: Create with category
ExpressionProfile-->>ProfileBuilder: Profile instance
ProfileBuilder-->>BuildingBlockCreator: Apply defaults
BuildingBlockCreator-->>Client: Return profile
else Species Invalid
SpeciesRepository-->>BuildingBlockCreator: Species not found
BuildingBlockCreator-->>Client: Throw ArgumentException
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/PKSim.R.Tests/BuildingBlockCreatorSpecs.cs (1)
56-70: Strengthen success-path assertions beyond non-null.These tests currently pass even if the wrong profile type/category/species gets serialized. Please deserialize or inspect the payload and assert expected fields (at least molecule, species, and profile type/category) per scenario.
Also applies to: 94-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/PKSim.R.Tests/BuildingBlockCreatorSpecs.cs` around lines 56 - 70, The test CreateExchangeMetabolizingEnzyme currently only asserts the returned string is non-null; update it to deserialize the returned payload from BuildingBlockCreator.CreateExpressionProfile(MetabolizingEnzyme, "CYP3A4", CoreConstants.Species.HUMAN) and assert that the resulting object has the expected molecule/name ("CYP3A4"), species (CoreConstants.Species.HUMAN or equivalent value), and profile type/category indicating a metabolizing enzyme; apply the same stronger assertions to the other similar tests that call BuildingBlockCreator.CreateExpressionProfile so each verifies molecule, species and profile type/category rather than just non-null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/PKSim.R/Exchange/BuildingBlockCreator.cs`:
- Around line 36-63: The code conflates "protein type" and "expression profile
category" by using the same parameter (category) to pick the generic T and then
writing it into expressionProfile.Category; fix by separating those concerns: in
CreateExpressionProfile compute a distinct proteinType selector (e.g. determine
which generic T to use based on category using the
TransportProtein/ProteinBindingPartner/MetabolizingEnzyme constants) but keep
the original input as profileCategory, then change createExpressionProfile<T> to
accept a profileCategory parameter (or overload it) and set
expressionProfile.Category = profileCategory (not the type label); update calls
to createExpressionProfile<T>(...) accordingly and ensure
expressionProfile.Category only contains the intended profile category string.
---
Nitpick comments:
In `@tests/PKSim.R.Tests/BuildingBlockCreatorSpecs.cs`:
- Around line 56-70: The test CreateExchangeMetabolizingEnzyme currently only
asserts the returned string is non-null; update it to deserialize the returned
payload from BuildingBlockCreator.CreateExpressionProfile(MetabolizingEnzyme,
"CYP3A4", CoreConstants.Species.HUMAN) and assert that the resulting object has
the expected molecule/name ("CYP3A4"), species (CoreConstants.Species.HUMAN or
equivalent value), and profile type/category indicating a metabolizing enzyme;
apply the same stronger assertions to the other similar tests that call
BuildingBlockCreator.CreateExpressionProfile so each verifies molecule, species
and profile type/category rather than just non-null.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/PKSim.Assets/PKSimConstants.cssrc/PKSim.R/Exchange/BuildingBlockCreator.cstests/PKSim.R.Tests/BuildingBlockCreatorSpecs.cs
Fixes #3421 Enable PK-Sim ExpressionProfile from MoBi.R
Description
Allow MoBi to call into PK-Sim to create an Expression profile. First is to create the default profile without the database query.
Type of change
Please mark relevant options with an
xin the brackets.How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Reviewer checklist
Mark everything that needs to be checked before merging the PR.
This change requires a documentation updateabove is selectedScreenshots (if appropriate):
Questions (if appropriate):
Summary by CodeRabbit
New Features
Chores