Add tests for combination logic with inclusion and washout periods in… - #157
Add tests for combination logic with inclusion and washout periods in…#157kirilklein wants to merge 1 commit into
Conversation
… OutcomeMaker * Introduced new test cases to validate the inclusion flag functionality for event combinations, ensuring correct outcomes based on specified time windows. * Implemented a test for washout periods to confirm that only the first occurrence of an event is captured when subsequent events occur within a defined timeframe. * Enhanced test coverage for the OutcomeMaker to ensure robust handling of complex event scenarios.
WalkthroughTwo new test methods were added to the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as TestOutcomeMaker
participant OutcomeMaker as OutcomeMaker
participant Data as Test Data
participant CSV as Output CSV
Test->>OutcomeMaker: Define combination outcome (with include/exclude)
OutcomeMaker->>Data: Run outcome logic on test data
OutcomeMaker->>CSV: Write filtered results to CSV
Test->>CSV: Read CSV results
Test->>Test: Assert correctness of filtered events
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/test_modules/test_outcomemaker.py (1)
1-773: Fix code formatting to pass CI checks.The pipeline indicates formatting issues that need to be addressed.
Run the following command to fix the formatting:
ruff format tests/test_modules/test_outcomemaker.py
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/test_modules/test_outcomemaker.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: kirilklein
PR: kirilklein/PHAIR_EHR#108
File: tests/test_main_causal/test_helper/test_select_cohort.py:39-43
Timestamp: 2025-06-07T13:06:20.414Z
Learning: When analyzing time window calculations in tests, be more careful with date arithmetic. Patient 3 with index date 2019-06-15 and data_end 2020-12-31 has approximately 565 days of follow-up, which exceeds the 365-day minimum requirement.
🪛 GitHub Actions: Format
tests/test_modules/test_outcomemaker.py
[error] 1-1: ruff formatting check failed. File would be reformatted. Run 'ruff format' to fix code style issues.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: pipeline_tests_performance
- GitHub Check: pipeline_tests
- GitHub Check: tests
- GitHub Check: Docstring Coverage
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: unittests
- GitHub Check: test
- GitHub Check: tests
- GitHub Check: Test Coverage
🔇 Additional comments (2)
tests/test_modules/test_outcomemaker.py (2)
701-734: LGTM! Well-structured test for inclusion flag behavior.The test correctly validates the default inclusion behavior (
exclude: false) for combination outcomes. The assertion logic properly expects patients 4 and 8 to be included (death within 7 days of MI) while excluding patient 9 (death 8 days after MI).
735-768: Excellent implementation of washout period logic!This test demonstrates an innovative use of the combination framework to implement washout periods. The logic correctly identifies first occurrences by excluding events that have prior occurrences within the specified timeframe. The test setup and assertions properly validate that Patient 6's first stroke and Patient 7's single stroke are included, while Patient 6's second stroke is excluded due to the washout period.
1 participant
… OutcomeMaker
Summary by CodeRabbit