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

Add some missing US Core checks now that we have attachment info #70

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

mikix
Copy link
Contributor

@mikix mikix commented Jan 9, 2025

Previously, the US Core checks around DiagnosticReport.presentedForm and DocumentReference.content.attachment were marked as "quirks" that we ignored due to the ETL stripping out the bits necessary to make those checks.

But now that the ETL leaves us enough info, we don't need to mark them as quirks and can just implement them.

This made the DocumentReference mandatory cube too large, however, so it's now split into two tables (like Observations are). So this is a breaking change for consumers, sorry.

Comment on lines -5 to -6
-- CUMULUS-QUIRK
-- Doesn't look for "presentedForm", because Cumulus strips that field.
Copy link
Contributor Author

@mikix mikix Jan 9, 2025

Choose a reason for hiding this comment

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

This was a copy-and-paste comment in error, afaict. DxReport Lab doesn't mark this field as must-support (it does mention it, but only in passing). DxReport Note does mark it as must-support, however (as seen in next file)

__version__ = "6.1.0"
__version__ = "7.0.0"
Copy link
Contributor Author

@mikix mikix Jan 9, 2025

Choose a reason for hiding this comment

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

Major bump because this PR splits the previous c_us_core_v4_count DocumentReference table into two (for CUBE performance reasons), which could affect any consumers. I've also bumped the data_package_version, which I think is a correct thing to do?

@mikix mikix force-pushed the mikix/docref-quirks branch 2 times, most recently from 3d7920a to 469d564 Compare January 9, 2025 12:33
@mikix mikix marked this pull request as ready for review January 9, 2025 12:37
Copy link

@dogversioning dogversioning left a comment

Choose a reason for hiding this comment

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

For posterity: we had an off-thread call about the mandatory_split parameter and where it's used - lowest hurdle for resolving that is a documentation update.

Previously, the US Core checks around DiagnosticReport.presentedForm
and DocumentReference.content.attachment were marked as "quirks" that
we ignored due to the ETL stripping out the bits necessary to make
those checks.

But now that the ETL leaves us enough info, we don't need to mark
them as quirks and can just implement them.

This made the DocumentReference mandatory cube too large, however,
so it's now split into two tables (like Observations are).
So this is a breaking change for consumers, sorry.
@mikix mikix force-pushed the mikix/docref-quirks branch from 469d564 to 37d3d3b Compare January 9, 2025 15:31
@mikix mikix merged commit 0cf12eb into main Jan 9, 2025
2 checks passed
@mikix mikix deleted the mikix/docref-quirks branch January 9, 2025 15:33
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.

2 participants