Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix export filter keeping old entries when there is an active entry of the same type #1385

Merged
merged 2 commits into from
Nov 10, 2023

Conversation

dehall
Copy link
Contributor

@dehall dehall commented Oct 26, 2023

Fixes #1383

This PR addresses an issue where the export filter isn't filtering out old entries when there is a separate active entry of the same "type", ie, code. This is especially likely to happen with "Medication Review Due" from the Med Rec module, but it could also happen for any potentially-recurring entry such as Viral Sinusitis or chronic medications.

The issue is that the logic is using record.conditionActive which only checks whether the given code is in the "active" set, it doesn't look at the specific entry at all. The change here is to only look at dates, and the date logic can be pretty simple: "if the entry is open (stop = 0) or the stop is after the cutoff, keep the entry"

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Merging #1385 (574152d) into master (6192ed7) will increase coverage by 0%.
The diff coverage is 80%.

@@           Coverage Diff            @@
##             master   #1385   +/-   ##
========================================
  Coverage        77%     77%           
- Complexity     3685    3704   +19     
========================================
  Files           170     170           
  Lines         24005   23997    -8     
  Branches       3331    3331           
========================================
+ Hits          18524   18587   +63     
+ Misses         4448    4387   -61     
+ Partials       1033    1023   -10     
Files Coverage Δ
...c/main/java/org/mitre/synthea/export/Exporter.java 62% <80%> (-1%) ⬇️

... and 21 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

Copy link
Member

@jawalonoski jawalonoski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether or not to approve this PR. On the one hand, it massively reduces the amount of bloat in the record due to repetition.... mostly related to entries like Medication review due (situation) and employment status...

But it also removes more valuable entries like Normal Pregnancy -- which is double-edged sword. We lose the ability to track how inaccurate our pregnancy model is (the reproduction rate is too high), but instead of fixing it in the model it just "looks better" because they get filtered in the output.

src/test/java/org/mitre/synthea/export/ExporterTest.java Outdated Show resolved Hide resolved
@dehall
Copy link
Contributor Author

dehall commented Nov 6, 2023

Hmm that's an interesting point. The thing is though we're already filtering those entries out today if there's not an active instance of the same type. This PR is just making the logic more consistent with how we intended it.
For the pregnancy stats, I would say we shouldn't be looking at filtered records anyway when we're reviewing data.

@jawalonoski jawalonoski merged commit c88eb6b into master Nov 10, 2023
6 checks passed
@jawalonoski jawalonoski deleted the export-filter-active-repeats branch November 10, 2023 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Export filter keeps all instances of a condition type when an active instance exists
2 participants