Add support for documentation generation for metrics classes. #1782

cmnbroad · 2022-03-08T16:10:16Z

@derekca @gbrandt6 @droazen @lbergelson FYI.

This implements documentation generation for metrics classes. It uses the existing @DocumentedFeature annotation, along with a custom doc template (which is easier than overloading the existing template), and a few lines of custom doclet code that selects that template for metrics classes and displays all public fields (in all super classes in the class hierarchy of the annotated metrics class) in a table.

There are two commits; one with the core code, and one that add the annotations to the various metrics classes. A separate sweep should probably be done to add metrics-specific summaries (using the @DocumentedFeature summary attribute) for each class, and also some of the metrics source classes seem to have no documentation at all.

Alternative implementations would be:

add a new class-level annotation to Barclay, analogous to @DocumentedFeature, and interpret it to mean "document all for non-@argument fields in a table", or
Add a new explicit field-level annotation to Barclay, analogous to @Argument, but that would require field-level annotation of each metric variable in each class

But the solution in this PR requires no Barclay changes, and given that the behavior seems pretty specific to metrics, I think its the simplest solution - its basically all custom code in Picard that specializes/overrides the default Barclay doclet. The one caveat is that if we want the GATK doc build to pick up the metrics doc as well, we'll need to propagate the new template and the few lines of custom doclet code to the GATK repo, since GATK uses it's own custom doclet and templates.

Below is some sample output (I don't have all the correct style sheets locally so these are not quite "publish-ready).

Index Page: (note that only AlignmentSummaryMetrics has a n actual summary, because its the only one I wrote - as mentioned above, all of the classes need to have a short summary line to be added):

Detail Page:

cmnbroad · 2022-03-14T16:18:56Z

@derekca Can you verify that the result is what you're looking for (I included sample screen shots, but I can give you the actual files if you want to look at them). Once we have that, we can move forward with the rest of the code review.

cmnbroad · 2022-03-14T18:09:38Z

src/main/resources/picard/helpTemplates/metrics.template.html

+		<h2>Overview</h2>
+		${description}
+
+			<#-- Create the argument summary -->


These are not arguments, their metrics....

cmnbroad · 2022-03-14T18:10:38Z

src/main/resources/picard/helpTemplates/metrics.template.html

+					<#list arguments.all as arg>
+						<@argumentDetails arg=arg/>
+					</#list>
+			</#if>


This if block and the comment are irrelevant and should be removed..

cmnbroad · 2022-03-14T18:13:24Z

src/main/java/picard/util/help/PicardHelpDocWorkUnitHandler.java

+                    final Map<String, String> metricsField = new HashMap();
+                    metricsField.put(METRICS_MAP_NAME_KEY, fd.name());
+                    metricsField.put(METRICS_MAP_SUMMARY_KEY, fd.getRawCommentText());
+                    metricsFields.add(metricsField);


Hm. Reviewing you own code days later is interesting. metricsFields -> metricsList and metricsField -> metricsFields.

derekca · 2022-03-15T16:53:14Z

@cmnbroad I'm not familiar enough with how the backend works, but what you're describing here is basically what we had in mind. I think that it should help us in migrating over the bulk of the information that we need onto the GATK repo.
Whatever else is leftover can be done manually by us in User Ed, if there is anything left over on the deprecated site.

gbggrant

LGTM. One very minor question.

gbggrant · 2022-03-16T18:33:54Z

src/main/java/picard/util/help/HelpConstants.java

@@ -43,6 +43,9 @@ private HelpConstants() {}
    public final static String DOC_CAT_GENOTYPING_ARRAYS_MANIPULATION = "Genotyping Arrays Manipulation";
    public final static String DOC_CAT_GENOTYPING_ARRAYS_MANIPULATION_SUMMARY = "Tools that manipulate data generated by Genotyping arrays";

+    public final static String DOC_CAT_METRICS = "Metrics";
+    public final static String DOC_CAT_METRICS_SUMMARY = "Metrics ";


Is the trailing space on "Metrics " intentional?

Oh, that space is entirely unnecessary. Fixed.

cmnbroad · 2022-03-21T14:14:24Z

@derekca

I think that it should help us in migrating over the bulk of the information that we need onto the GATK repo.

Not sure exactly what this refers to. Is the plan to restart the publishing of the Picard doc along with Picard releases (I assume we're still not doing that), or to just continue to publish the output of the GATK doc build ? In order for the metrics doc to be produced by the GATK doc build, a separate PR (a subset of this one) will be required in GATK to enable these there. Once this is merged, I'll add that PR to GATK.

derekca · 2022-03-29T15:40:22Z

@cmnbroad

Is the plan to restart the publishing of the Picard doc along with Picard releases (I assume we're still not doing that)

Basically, we will be shutting down the Picard site. There is no set date for the site to be shut down, but it will happen as soon as we finish migrating all the information from that site into GATK documentation as part of this ticket.

So, any new Picard documentation or updates should go directly to the GATK docs, and not to the site.

or to just continue to publish the output of the GATK doc build ?

Yes, exactly. The GATK docs will continue to get built, but now they can pull the [missing] Picard metrics information as well.

In order for the metrics doc to be produced by the GATK doc build, a separate PR (a subset of this one) will be required in GATK to enable these there. Once this is merged, I'll add that PR to GATK.

Okay, if that makes the most sense, then let's do that!

cmnbroad · 2022-03-29T21:22:18Z

Oh I see derekca - that sounds good. I'll need to rebase this now since its out-of-date and then we can merge it so I can make the GATK PR.

… added.

gbggrant · 2022-03-30T19:05:16Z

@cmnbroad is this PR ready to merge?

cmnbroad · 2022-04-04T13:00:49Z

src/main/java/picard/util/help/PicardHelpDocWorkUnitHandler.java

+            final FieldDoc[] fieldDocs = currentWorkUnit.getClassDoc().fields(false);
+            for (final FieldDoc fd : fieldDocs) {
+                if (fd.isPublic()) {
+                    final Map<String, String> metricsFields = new HashMap();


The stricter GATK build complains about this raw type. Fixed.

cmnbroad · 2022-04-04T13:03:06Z

I updated one line due to fix usage of a raw generic type that was detected by the GATK build when I made the GATK pr to reflect these changes. This is ready once tests pass.

cmnbroad marked this pull request as draft March 8, 2022 16:11

cmnbroad commented Mar 14, 2022

View reviewed changes

cmnbroad marked this pull request as ready for review March 15, 2022 15:23

gbggrant requested a review from derekca March 16, 2022 17:20

gbggrant assigned derekca Mar 16, 2022

gbggrant approved these changes Mar 16, 2022

View reviewed changes

gbggrant assigned cmnbroad Mar 16, 2022

cmnbroad force-pushed the cn_metrics_doc branch from f88ed99 to fbf97d8 Compare March 29, 2022 21:23

cmnbroad added 5 commits March 29, 2022 17:24

Add support for documentation generation for metrics classes.

63ebe99

Annotate metrics classes for docgen. These still need summaries to be…

dff098d

… added.

Respond to my own comments.

4f418f2

Dont annotate MultiLevelMetrics.

6200c70

Remove unnecessary space.

fbf97d8

cmnbroad mentioned this pull request Apr 4, 2022

Mirror of Picard metrics documentation generation changes. broadinstitute/gatk#7749

Merged

cmnbroad commented Apr 4, 2022

View reviewed changes

Fix usage of a raw type detected by the stricter GATK build.

b373e05

cmnbroad merged commit 5854951 into master Apr 4, 2022

cmnbroad deleted the cn_metrics_doc branch April 4, 2022 13:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for documentation generation for metrics classes. #1782

Add support for documentation generation for metrics classes. #1782

cmnbroad commented Mar 8, 2022 •

edited

Loading

cmnbroad commented Mar 14, 2022

cmnbroad Mar 14, 2022

cmnbroad Mar 21, 2022

cmnbroad Mar 14, 2022

cmnbroad Mar 21, 2022

cmnbroad Mar 14, 2022

cmnbroad Mar 21, 2022

derekca commented Mar 15, 2022

gbggrant left a comment

gbggrant Mar 16, 2022

cmnbroad Mar 21, 2022

cmnbroad commented Mar 21, 2022

derekca commented Mar 29, 2022 •

edited

Loading

cmnbroad commented Mar 29, 2022

gbggrant commented Mar 30, 2022

cmnbroad Apr 4, 2022

cmnbroad commented Apr 4, 2022

Add support for documentation generation for metrics classes. #1782

Add support for documentation generation for metrics classes. #1782

Conversation

cmnbroad commented Mar 8, 2022 • edited Loading

cmnbroad commented Mar 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derekca commented Mar 15, 2022

gbggrant left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmnbroad commented Mar 21, 2022

derekca commented Mar 29, 2022 • edited Loading

cmnbroad commented Mar 29, 2022

gbggrant commented Mar 30, 2022

Choose a reason for hiding this comment

cmnbroad commented Apr 4, 2022

cmnbroad commented Mar 8, 2022 •

edited

Loading

derekca commented Mar 29, 2022 •

edited

Loading