DM-51162: Add histograms to wholeSkyPlot #442

sr525 · 2025-11-19T22:17:04Z

No description provided.

sr525 · 2025-11-20T19:45:30Z

python/lsst/analysis/tools/actions/plot/wholeSkyPlot.py

    yLimits = ListField[float](doc="Plotting limits for the y axis.", default=[-10.0, 60.0])
    autoAxesLimits = Field[bool](doc="Find axes limits automatically.", default=True)
-    dpi = Field[int](doc="DPI size of the figure.", default=500)
-    figureSize = ListField[float](doc="Size of the figure.", default=[9.0, 3.5])


I have removed these because the figures are getting very large and taking a long time to get from the butler. The figure size now also needs to be very specific and if you change it nothing will line up. If this becomes a problem for @erinleighh then I will make a better fix on another ticket in the future.

jrmullaney

Generally looks fine.
I wonder whether there should be catches in case both low and high thresholds aren't specified. It could be that one always needs to specify lowThreshold and highThreshold in the yaml file, but I think the code should be able to handle one of them being None or Nan or something similar to indicate that it hasn't got a lower/upper threshold.

jrmullaney · 2025-11-24T13:12:24Z

python/lsst/analysis/tools/actions/plot/histPlot.py

        default=None,
    )
+    addThreshold = Field[bool](
+        doc="Read in the predefined threshold and add it to the histogram.",


Should this be "threshold(s)"? Aren't there often more than one thresholds per metric?
Also, suggest "indicate it on the histogram" rather than "add".

jrmullaney · 2025-11-24T13:22:02Z

python/lsst/analysis/tools/actions/plot/histPlot.py

                    ax.axvline(meds[i], ls=(0, (5, 3)), lw=1, c=colors[i])
+                    if self.panels[panel].addThreshold and hist in metricDefs:
+                        lowThreshold = metricDefs[hist]["lowThreshold"]
+                        highThreshold = metricDefs[hist]["highThreshold"]


Does this assume that there will always be a "lowThreshold" and a "highThreshold" for each metric in the yaml file.
Will that always be the case?
Will it break meaningfully if someone doesn't include either a "lowThreshold" or a "highThreshold" when they add a new metric to the metricInformation.yaml file, or do we want it to be ok with adding just a single line?

I would prefer if people always added a sensible value for both however I have added catches for infinite values and only one of the thresholds, or neither, being present.

jrmullaney · 2025-11-24T13:24:33Z

python/lsst/analysis/tools/actions/plot/wholeSkyPlot.py

    labelTracts = Field[bool](doc="Label the tracts.", default=False)

+    addThreshold = Field[bool](
+        doc="Read in the predefined threshold and add it to the histogram.",


Same comment as for the same config in histPlot.py.

Suggested change

doc="Read in the predefined threshold and add it to the histogram.",

doc="Read in the predefined thresholds and indicate them on the histogram.",

jrmullaney · 2025-11-24T13:25:30Z

python/lsst/analysis/tools/actions/plot/wholeSkyPlot.py

        -------
        text : `str`
            A string containing the 5 tracts with the largest outlier values.
        """


Should this say "10 tracts" now?

jrmullaney · 2025-11-24T13:26:02Z

python/lsst/analysis/tools/actions/plot/wholeSkyPlot.py

            # Sort values in descending (-) absolute value order discounting
            # NaNs.
            maxInds = np.argsort(-np.abs(outlierValues))
            # Show up to five values on the plot.


Should this say "up to ten values" now?

jrmullaney · 2025-11-24T14:02:42Z

python/lsst/analysis/tools/actions/plot/wholeSkyPlot.py

+        dataName = self.zAxisLabel.format_map(kwargs)
+        colBarVals = np.array(colBarVals)
+        if self.addThreshold and dataName in metricDefs:
+            lowThreshold = metricDefs[dataName]["lowThreshold"]


Again, just wondering whether there should be a catch in cases where there isn't both a lower and upper threshold in metricInformation.
Or what happens if one of them is Nan or None.

jrmullaney · 2025-11-24T14:06:29Z

python/lsst/analysis/tools/actions/plot/wholeSkyPlot.py

+        ax1.yaxis.tick_right()
+
+        if self.addThreshold and dataName in metricDefs:
+            ax1.axvline(lowThreshold, color=accent_color())


Again, will we always have upper and lower thresholds?
What if one of these is Nan or None?

I've caught this but if people go around setting the thresholds to NaN I will be irritated.

python/lsst/analysis/tools/metricInformation.yaml

jrmullaney

Just a couple of typos - you can just accept my suggestions on github, if you like.

jrmullaney · 2025-11-26T10:11:29Z

python/lsst/analysis/tools/actions/plot/histPlot.py

        default=None,
    )
+    addThresholds = Field[bool](
+        doc="Read in the predefined threshold and indicate it on the histogram.",


Suggested change

doc="Read in the predefined threshold and indicate it on the histogram.",

doc="Read in the predefined thresholds and indicate them on the histogram.",

jrmullaney · 2025-11-26T10:12:43Z

python/lsst/analysis/tools/actions/plot/wholeSkyPlot.py

    labelTracts = Field[bool](doc="Label the tracts.", default=False)

+    addThreshold = Field[bool](
+        doc="Read in the predefined threshold and add it to the histogram.",


Suggested change

doc="Read in the predefined threshold and add it to the histogram.",

doc="Read in the predefined thresholds and indicate them on the histogram.",

…d error catching

sr525 force-pushed the tickets/DM-51162 branch 15 times, most recently from 8694c7b to 33b42d4 Compare November 20, 2025 19:42

sr525 commented Nov 20, 2025

View reviewed changes

jrmullaney self-requested a review November 24, 2025 13:08

jrmullaney requested changes Nov 24, 2025

View reviewed changes

sr525 force-pushed the tickets/DM-51162 branch 3 times, most recently from f6e24a3 to 08b72d8 Compare November 24, 2025 23:18

jrmullaney approved these changes Nov 26, 2025

View reviewed changes

sr525 force-pushed the tickets/DM-51162 branch from 08b72d8 to 2eff89c Compare November 26, 2025 19:11

sr525 added 4 commits December 1, 2025 08:23

Change histogram plotting code to work with metrics

600a07e

Add a yaml file of metric thresholds

9b839a7

Display error messages when a task fails

ad75bac

Add histograms to wholeSkyPlot, bonus speed ups, metric thresholds an…

d8cc047

…d error catching

sr525 force-pushed the tickets/DM-51162 branch from 2eff89c to d8cc047 Compare December 1, 2025 16:23

sr525 merged commit 20a5142 into main Dec 1, 2025
13 checks passed

sr525 deleted the tickets/DM-51162 branch December 1, 2025 17:35

	doc="Read in the predefined threshold and add it to the histogram.",
	doc="Read in the predefined thresholds and indicate them on the histogram.",

DM-51162: Add histograms to wholeSkyPlot #442

DM-51162: Add histograms to wholeSkyPlot #442

Uh oh!

Conversation

sr525 commented Nov 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrmullaney left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jrmullaney left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jrmullaney left a comment •

edited

Loading