Stream: genomics/committers
Topic: Grouperless
Kevin Power (Nov 23 2021 at 21:43):
Seems like we need to take more advantage of the committers stream to discuss more technical detail, so starting a thread to talk about 'Grouperless' branch work.
CC: @Bret H @Jamie Jones
Bret H (Nov 23 2021 at 21:45):
Great!
Bret H (Nov 24 2021 at 17:54):
I pushed some changes that are not complete. I am still working on the branch, but it will have some bad pages at the moment
Bret H (Nov 24 2021 at 18:22):
still tweaking, want it to be prettier
Bret H (Nov 24 2021 at 18:38):
I have to drop off soon but will return this evening. I'll do a push this evening with the final changes and note when it is pushed here.
Bret H (Nov 24 2021 at 19:44):
not done but have a peek at Appendix H: Grouping Guidance
Kevin Power (Nov 24 2021 at 20:26):
BTW - Looks like your build failed @Bret H
Kevin Power (Nov 24 2021 at 20:26):
https://chat.fhir.org/#narrow/stream/179297-committers.2Fnotification/topic/ig-build/near/262633232
Bret H (Nov 25 2021 at 00:46):
Error connecting to build server - running without build (class com.google.gson.JsonNull cannot be cast to class com.google.gson.JsonArray (com.google.gson.JsonNull and com.google.gson.JsonArray are in unnamed module of loader 'app'))
Bret H (Nov 25 2021 at 00:46):
I'm going to build again.
Bret H (Nov 25 2021 at 00:47):
it built locally and I had updated the publisher
Bret H (Nov 25 2021 at 00:52):
What's going on here? "Publishing Content Failed: Error loading template hl7.fhir.template#current: The package 'hl7.fhir.template' has no entry on the current build server (00:21.0404)"
I'm updating the publisher again locally.
Bret H (Nov 25 2021 at 01:10):
so...not sure why but I pushed slnx.sqlite and we're good now
Bret H (Nov 25 2021 at 01:23):
I am still having a problem with linking to http://hl7.org/fhir/R4/diagnosticreport.html#10.3.5.3
Trying to use {{site.data.fhir.path}}diagnosticreport.html#10.3.5.3 but it is not working.
<a ref="{{site.data.fhir.path}}diagnosticreport.html#10.3.5.3">Associated Observations</a>
Bret H (Nov 25 2021 at 01:37):
trying unicode on the #
Bret H (Nov 25 2021 at 01:41):
oh the joys. the actual rendered page has
<a ref="http://hl7.org/fhir/R4/diagnosticreport.html#10.3.5.3">Associated Observations</a>
and
<a ref="http://hl7.org/fhir/R4/observation.html#obsgrouping">Observation Grouping</a>
which should be right.....
Bret H (Nov 25 2021 at 04:04):
following works in some pages: <a href="https://www.hl7.org/fhir/diagnosticreport.html#10.3.5.3">Diagnostic Report Resource</a>
Any way. I'll finalize shortly. but would like to know if you like what's been done to the figures in terms of layout. After some spins, I took advantage of the bootstrap and jquery libraries that the publisher uses with a few class tricks and divs.
Bret H (Nov 25 2021 at 06:02):
about to publish.
Bret H (Nov 25 2021 at 06:24):
please check grouping observations page, Table of contents, Dropdown in header bar, index, general and query pages (fyi Iterating through has-member and derived-from and a sentence at the top #3 are most important for the query page)
http://build.fhir.org/ig/HL7/genomics-reporting/branches/withoutGrouper/groupingGuidance.html
http://build.fhir.org/ig/HL7/genomics-reporting/branches/withoutGrouper/toc.html
http://build.fhir.org/ig/HL7/genomics-reporting/branches/withoutGrouper/index.html#understanding-fhir
http://build.fhir.org/ig/HL7/genomics-reporting/branches/withoutGrouper/index.html#appendices
http://build.fhir.org/ig/HL7/genomics-reporting/branches/withoutGrouper/general.html#genomics-report (be sure to read the whole section, especially 'Genomic Report Overview')
http://build.fhir.org/ig/HL7/genomics-reporting/branches/withoutGrouper/general.html#referencing-observations-from-genomicsreport
http://build.fhir.org/ig/HL7/genomics-reporting/branches/withoutGrouper/general.html#querying
http://build.fhir.org/ig/HL7/genomics-reporting/branches/withoutGrouper/usecases.html#appendix-d-query-guidance
http://build.fhir.org/ig/HL7/genomics-reporting/branches/withoutGrouper/usecases.html#iterating-through-has-member-and-derived-from
let me know if you have edits or comments or questions. @Kevin Power @Jamie Jones
Bret H (Nov 25 2021 at 06:28):
fyi: https://chat.fhir.org/#narrow/stream/179297-committers.2Fnotification/topic/ig-build/near/262670877 most of the warnings are terminology and were resident before the branch was created. The broken link is to Observation resource.
I think it is ready. But would be great to have your confirmation and spotting of typos.
Kevin Power (Nov 25 2021 at 14:41):
Good stuff Bret! My first quick review feedback:
Index page - looks like we lost the link to the Diagnostics module?
Grouping guidance- looks like we still need the cleaned up images from @Jamie Jones
Query - seems like the panel section is missing and example query?
Bret H (Nov 25 2021 at 16:08):
Query - http://build.fhir.org/ig/HL7/genomics-reporting/branches/withoutGrouper/usecases.html#iterating-through-has-member-and-derived-from @Kevin Power
Bret H (Nov 25 2021 at 16:11):
index section - The diagnostics module is linked to twice in the below. Did you mean a link on the phrase 'Diagnostics Module' in italics below, as well as the two other links? @Kevin Power
from http://build.fhir.org/ig/HL7/genomics-reporting/branches/withoutGrouper/index.html#understanding-fhir
"
It is highly recommended that readers review Diagnostics Module and review the resources that are used as part of this implementation guide, especially Observation, DiagnosticReport and MolecularSequence.
Importantly, this guide heavily uses Observation and DiagnosticReport as described in the Diagnostics Module. Implementers should pay close attention to the considerations in Associated Observations and Observation Grouping describing proper usage of the relationships “diagnosticReport.result”, “Observation.hasMember”, and “Observation.derivedFrom”. Consumers of Genomic diagnostic reports MUST navigate through all hasMember relations, and navigate through derivedFrom relationships in order to ensure all clinically relevant information is seen.
"
Bret H (Nov 25 2021 at 16:13):
@Kevin Power did you mean this section in query? http://build.fhir.org/ig/HL7/genomics-reporting/branches/withoutGrouper/usecases.html#panel
Bret H (Nov 25 2021 at 16:16):
it'd be a query using Observation.code and also the iteration through hasMember and derivedFrom to get all the Panel components - can through a patient on there. However, if we don't reference a specific panel then the query would return all instances of that panel, and one would need to use the links to understand which panel result has which specific instances of variant observations.
You'd need logic to parse that result...oh, and I did not put in an example of iteration without using 'iterate' operator --- and I need to add a note from the search page about a caution that a server might protect itself and not infinitely follow references.
Bret H (Nov 25 2021 at 16:17):
(the WG does good work)
Kevin Power (Nov 25 2021 at 17:18):
Index page - yes that is the missing link. I am pretty sure it was a link in the previous version?
Query - yes that is the section. Maybe I didn’t have an example before, but I thought it was a section you might have added.
Bret H (Nov 25 2021 at 19:41):
Great! will address both. Anything else?
Bret H (Nov 26 2021 at 01:43):
hmmm....this panel has quite a few variants : ^ ) I'm going to look for a panel with slightly less variants e.g. https://blueprintgenetics.com/tests/panels/nephrology/nephrotic-syndrome-panel/
Bret H (Nov 26 2021 at 01:44):
the point is to use a panel. I'd like to do a complete panel
Bret H (Nov 26 2021 at 01:47):
geesh GTR really rocks as a resource for genetic tests. CHeck this out: https://www.ncbi.nlm.nih.gov/gtr/tests/509600/interpretation/
It even gives info on software used!
Bret H (Nov 26 2021 at 02:10):
gotta go eat Turkey. I'm putting in a query example, but without an example profile to go with it. Basically, the observation.code parameter is used to find the panel then the iterative parameters are used.
Bret H (Nov 26 2021 at 03:35):
Published :tada:
Bret H (Nov 26 2021 at 03:42):
The panel example could be commented out, but it's at http://build.fhir.org/ig/HL7/genomics-reporting/branches/withoutGrouper/usecases.html#panel
Kevin Power (Nov 26 2021 at 15:09):
Made a quick fix to comment out some leftover text in the panel section.
Kevin Power (Nov 30 2021 at 19:33):
I have created a PR to get final reviews of this branch before we merge:
https://github.com/HL7/genomics-reporting/pull/49
Kevin Power (Nov 30 2021 at 20:05):
@Bret H corrected a merge conflict, and I am working on a few quick QA fixes.
Jamie Jones (Dec 01 2021 at 15:59):
@Bret H I'm reviewing the pull request and I wanted to ask if we thought it may be better to update the grouped report examples to not use grouping, rather than leave them in and add new example artifacts that are ungrouped, given Lloyd's recent comments on new artifacts added since ballot being flagged as draft instead of STU. Though maybe 'draft' is more appropriate, I don't want to have so many examples reinforcing a pattern we are trying to discourage.
Specifically, I'd suggest replacing "DiagnosticReport/PGxGenomicsReportEMERGE" with the contents of "DiagnosticReport/PGxGenomicsReportEMERGE2",
"Bundle/oncologyexamples-r4" with the contents of "Bundle/oncologyexamples-r4-noGrouping", and "Bundle/diagnosticreport-cgexample" with the contents of "Bundle/diagnosticreport-cgexample-noGroup", and update where they are linked from the text. Thoughts?
Bret H (Dec 01 2021 at 16:16):
@Jamie Jones I already created ungrouped versions of examples, they are there.
The vote was on the one with the grouped examples. The spirit of the vote was that the grouper profile be removed, that grouping not be prohibited, guidance on handling grouping be given, and, regards, EMERGE the use of grouping there reflects the organization that Mullai et al agreed with.
For the EMERGE one, I would like Mullai to comment. @Mullai Murugan would you like the EMERGE example to not use grouping? (I've got an example already made, and a figure exists without the grouping).
For the non-EMERGE examples, they exist to illustrate examples of how to create reports etc. I think it would be within the voted upon opinion to change the links to point to the grouping-less versions. But it would have been better to have this mentioned prior to the vote. I'll change those links now. BTW I think it is a good move.
@Kevin Power if you have not merged yet, please wait till I change the links. Thanks!
Kevin Power (Dec 01 2021 at 16:46):
@Bret H - Will hold off on the merge for now, no problem.
Bret H (Dec 01 2021 at 16:48):
thanks. I'm looking forward to moving on from the branch very soon
Jamie Jones (Dec 01 2021 at 16:49):
Thanks Bret, I think it was very helpful to view the grouperless examples alongside the older ones! I just realized that we didn't explicitly decide to keep the older ones and think removing them would be clearer going forward
Jamie Jones (Dec 01 2021 at 16:50):
and we will have to resolve the warning message about new artifacts in jira.xml if they use new names
Jamie Jones (Dec 01 2021 at 16:50):
so if we can reuse the old names it would be best :)
Bret H (Dec 01 2021 at 16:50):
Yep
Bret H (Dec 01 2021 at 16:51):
regards EMERGE. Can we update that separately in the main branch? not sure when Mullai will be able to comment. But I guess there is no hurry at the moment.
Jamie Jones (Dec 01 2021 at 16:55):
mullai did second the motion to "Update the PGx guidance to match up with the new guidance" which on the eMERGE grouping tracker, so should be good to go ahead with it as ungrouped (of note, it was never in our IG in a way that was faithful to their original use of grouper anyways)
Kevin Power (Dec 01 2021 at 16:55):
I concur (FWIW)
Jamie Jones (Dec 01 2021 at 16:56):
they had used grouper to split out the PGx results from the Dx results, we only modeled the PGx half of their report
Kevin Power (Dec 01 2021 at 17:27):
Just to confirm - @Bret H do you have time to make that change today?
-edit: At worst case, we have it in the branch history and can go back to it if we change our mind later.
Bret H (Dec 01 2021 at 17:29):
I can change all but EMERGE
Bret H (Dec 01 2021 at 17:47):
@Jamie Jones sorry. We need to keep the grouping example oncology bundle. it is used in the grouping guidance page.
Jamie Jones (Dec 01 2021 at 17:48):
that is fine, just wanted to minimize the nearly duplicate examples
Bret H (Dec 01 2021 at 17:48):
they are not duplicates
Kevin Power (Dec 01 2021 at 18:40):
OK, so I maybe missing it now, but does all this mean we have other changes to make or not?
Bret H (Dec 01 2021 at 18:46):
I removed commented out guidance as a clean-up.
We're using the oncology bundle in the guidance at the moment. So I think we should keep it, for the moment.
For EMERGE, I think once Mullai comments we can change or keep. But I'd prefer to leave it as is currently in the branch.
For the CG example, I could not find it referred to on anything other than the artifact page. I think we could switch to the without grouping version without causing a problem (remove the current cg-example and rename the cg-example-without grouper). ok? After that can we call it good?
Jamie Jones (Dec 01 2021 at 19:29):
I'm fine with that, can tweak them after the merge as well if that's when we get consensus.
Kevin Power (Dec 01 2021 at 19:30):
I suppose we can keep the examples with the groupings, but it seems like they should be the new files. So we could do the following:
- swapping the content between the files
- renaming each "-noGrouping" file to "-withGrouping"
- update the links from our guidance pages to the appropriate files
If we do that, we can keep all the examples @Bret H did, but keep the original files more in alignment with our guidance, and have the new files show what is looks like if someone does their own grouping.
Jamie Jones (Dec 01 2021 at 19:34):
Jamie Jones said:
they had used grouper to split out the PGx results from the Dx results, we only modeled the PGx half of their report
to clarify, the eMERGE grouping as it went to ballot in the IG currently was arbitrarily made up by me
Bret H (Dec 01 2021 at 19:42):
thanks Jamie
Bret H (Dec 01 2021 at 19:59):
Thanks Kevin.
Give me an hour. should be done soon
Bret H (Dec 01 2021 at 21:04):
decided to rename parts of the bundles as well...slow going. need to drop off will be back on later this evening.
Bret H (Dec 01 2021 at 22:40):
back to the branch. building locally
Bret H (Dec 01 2021 at 22:46):
1 error. Warnings down from 78 to 5
Bret H (Dec 01 2021 at 22:48):
one reference to fix
Kevin Power (Dec 01 2021 at 22:48):
With the 'File Attachments' branch, I added some additional warnings to the 'ignore' file.
Bret H (Dec 01 2021 at 22:50):
ok. building again
Bret H (Dec 01 2021 at 23:00):
while waiting. I have not dived into it but "WARNING: CodeSystem/condition-inheritance-mode-cs: CodeSystem: HL7 Defined CodeSystems SHOULD have a stated value for the caseSensitive element so that users know the status and meaning of the code system clearly
== fsh-generated\resources\CodeSystem-sequence-phase-relationship-cs.json =="
is a field for status missing from the FSH file?
Jamie Jones (Dec 01 2021 at 23:03):
Yeah, new requirement we haven't addressed yet
Kevin Power (Dec 01 2021 at 23:05):
Looks like you could add it like this:
- ^caseSensitive = true
Bret H (Dec 01 2021 at 23:06):
leave it for other branches or put it in now?
Kevin Power (Dec 01 2021 at 23:14):
For stuff like this, I happen to be a 'fix it when you find it' and this feels like a safe change to me, but I don't know, what do others think?
Jamie Jones (Dec 01 2021 at 23:16):
This will be main branch soon so go for it
Bret H (Dec 01 2021 at 23:16):
Will do
Bret H (Dec 01 2021 at 23:24):
just to be pedantic "* ^caseSensitive = true" gotta have that asterisk...I checked and all looked good. Added the caseSensitive FSH code and am building locally again. SUSHI passed but waiting for process to end
Kevin Power (Dec 01 2021 at 23:26):
Yea sorry, copy paste turned my "*" into a bullet
Bret H (Dec 01 2021 at 23:30):
figured, but thought to add the asterisk explicitly in the chat in case someone wanders into this chat.
Bret H (Dec 01 2021 at 23:40):
down to one warning about jira specifications and XML folder.
5 Infos - 3 for use of NCIT url ending with 'owl' which we have to live with till the URI is established in the terminology threads.
Bret H (Dec 01 2021 at 23:41):
2 info related to DiagnosticReport.code for the EMERGE example, any reason for code it is currently using?
Bret H (Dec 01 2021 at 23:44):
FYI I committed the changes
Kevin Power (Dec 01 2021 at 23:44):
Since they are info level, probably OK.
Bret H (Dec 01 2021 at 23:44):
I pushed to the branch
Bret H (Dec 01 2021 at 23:52):
@Kevin Power @Jamie Jones @Patrick Werner pushed to branch after Jamie review, published branch. Next step?
Kevin Power (Dec 02 2021 at 14:22):
I have merged to master! Many thanks to everyone, but especially @Bret H for all his hard work on this one.
https://media2.giphy.com/media/IwAZ6dvvvaTtdI8SD5/giphy.gif?cid=c623cb350b7liios0gfpmueh2tgb7jyibd8ifaps0ionfp07&rid=giphy.gif&ct=g
Kevin Power (Dec 02 2021 at 14:27):
And just FYI, I normally delete the branch after merging but I am going to let it sit for a few days in case we find we missed something.
Last updated: Apr 12 2022 at 19:14 UTC
