DM-51705: Assorted bugfixes, add mypy coverage, cleanup docstrings#75
Merged
Conversation
fred3m
approved these changes
Apr 28, 2026
fred3m
left a comment
fred3m
left a comment
There was a problem hiding this comment.
LGTM. Fixing one bug per commit, with good descriptions is a clear win here. I'm definitely motivated to do this with scarlet lite and meas_extensions_scarlet now.
Comment on lines
+75
to
+93
| ┌──────────────────────────────┐ | ||
| │ summit_extras │ ← This package | ||
| │ (analysis, plotting, tools) │ | ||
| └──────────┬───────────────────┘ | ||
| │ imports | ||
| ┌──────────▼───────────────────┐ | ||
| │ summit_utils │ ← Utility primitives (bestEffortIsr, | ||
| │ (data access, EFD, etc.) │ EFD client wrappers, camera utils) | ||
| └──────────┬───────────────────┘ | ||
| │ imports | ||
| ┌──────────▼───────────────────┐ | ||
| │ LSST Science Pipelines │ ← afw, daf_butler, ip_isr, | ||
| │ (core DM stack) │ meas_algorithms, pipe_tasks, etc. | ||
| └──────────┬───────────────────┘ | ||
| │ imports | ||
| ┌──────────▼───────────────────┐ | ||
| │ Third-party (numpy, scipy, │ | ||
| │ matplotlib, astropy, etc.) │ | ||
| └──────────────────────────────┘ |
There was a problem hiding this comment.
I find this diagram a little confusing I would expect the import arrow to go in the other direction. Also, is it necessary to have numpy, scipy, etc, on there? This is being nitpicky but I think this is unnecessary.
Contributor
Author
There was a problem hiding this comment.
I kind of agree, certainly about the arrows. I kind of see the point of the lowest box, but could go either way on it. But here's the pretty messed up thing here (IMO, at least): these are for Claude as well as being by Claude, so if it makes sense to it, it should stay, IMO. 1) what do you think of that, and 2) if you're convinced by that, how does that make you feel?!
There was a problem hiding this comment.
I'm not hugely concerned either way. I feel like 3rd party imports in a python package is a given, and I'm not sure that there's any utility to keeping it, even for Claude. I also don't think that if the arrows were in the other direction that it would make much of a difference to Claude. I guess you could make the argument that if you switched the arrow order then having the 3rd party packages makes the direction that it flows more explicit because they are obviously being imported by the boxes above.
Comment on lines
+470
to
+471
| maxMovementX = np.max(np.abs(dx)) | ||
| maxMovementY = np.max(np.abs(dy)) |
Contributor
Author
There was a problem hiding this comment.
Not the only one either, I think! (Some of the other might have been in other packages though)
mfisherlevine
force-pushed
the
tickets/DM-51705
branch
from
30c16d2 to
f717d10
Compare
Add return type annotations, parameter type annotations, and fix incorrect type annotations (e.g. np.array -> np.ndarray) to improve mypy coverage across all modules and the plotting subpackage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add missing module stubs to mypy.ini (ipdb, treegp, lsst.afw.geom.ellipses, lsst.meas, lsst.summit.extras.slewTiming) - Fix incorrect return types (_generateLegendText, _getBboxes, keyValuesSetFromFiles, getAxisName) - Add type annotations for local variables where mypy requires them (fitData, headersDict, dataDict, kValues, joinedValues, kwargs) - Fix real bug: _printLineIf.print(line) -> _printLineIf(line) - Fix ZoneInfo usage: replace pytz-style .localize() with .replace(tzinfo=) - Fix matplotlib type stub issues with type: ignore comments for axvline datetime args and upstream Butler type mismatches - Use plt.get_cmap() instead of plt.cm.viridis / cm.rainbow for compatibility with matplotlib type stubs - Fix list variance issues in fastStarTrackerAnalysis by using proper union types and type: ignore where list invariance prevents clean typing - Add missing return statement to getAxisName (raises ValueError) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Prompt:
Go over every function and class in the file and make sure that all
the docstrings are up to date, correctly formatted, and accurate and
descriptive.
Authored by Claude 4.6 Opus (high-effort)
In COMMANDS_TO_QUERY, the line for `logevent_slewControllerSettings` was missing a trailing comma. Python's implicit string concatenation merged it with the next entry (`logevent_startIntegration`), producing a single garbage topic name of the two joined together. As a result both the M1M3 slew-controller-settings event and the MTCamera startIntegration event were silently absent from every slew-timing plot the Simonyi version produced. Written by Claude Code 4.6 Opus (high-effort).
The threshold expression `bgMean + 0 * bgStd` multiplied the nSigma-weighted standard deviation by zero, so the `nSigma` argument was silently ignored and every pixel above the clipped mean was counted as an over-threshold pixel regardless of the user's setting (default 15). Replace the `0` with `nSigma` so the documented behaviour is actually applied. Written by Claude Code 4.6 Opus (high-effort).
The consistency check took `np.max(dx)` and `np.max(dy)` on signed deltas, which returns the largest *positive* step. A star drifting monotonically in one direction produces all-negative diffs, and the reported 'maximum movement' is then the least-negative value, which compares below `maxAllowableShift` no matter how far the star moved. Use `np.max(np.abs(...))` so the check reflects the magnitude of the largest inter-frame displacement. Written by Claude Code 4.6 Opus (high-effort).
The coma amplitude was computed with `np.hypot(meanComa[2], meanComa[2])`, which reduces to `abs(coma2) * sqrt(2)` and ignores the coma1 component entirely. The coma1/coma2 pair is a spin-1 vector whose amplitude is `sqrt(coma1**2 + coma2**2)`, matching the adjacent `np.arctan2(meanComa[2], meanComa[1])` convention. Replace the duplicate with `np.hypot(meanComa[1], meanComa[2])`. Written by Claude Code 4.6 Opus (high-effort).
The single colorbar attached to the e2 panel was labelled just `"e"`, which reads as 'total ellipticity' while actually showing e2 values. Because the e1 and e2 scatters share `vmin=-emax, vmax=emax` and the `"bwr"` cmap, the bar also applies visually to the e1 panel next to it. Relabel as `"e1, e2"` so the single bar is correctly identified as covering both components. Written by Claude Code 4.6 Opus (high-effort).
The scatter coordinates at lines 130-131 use `cornersDeg[:, 0]` as `x`
and `cornersDeg[:, 1]` as `y`, following the standard convention, but
the `xlabel`/`ylabel` calls at the end of the function had them the
wrong way round ('Field Angle Y' on the x-axis, 'Field Angle X' on the
y-axis). This silently mislabelled every focal-plane FWHM plot the
function produced. Swap the labels so each axis is named correctly.
Written by Claude Code 4.6 Opus (high-effort).
`makeFocalPlaneFWHMPlot` takes both `fig` and `ax` parameters but called
`plt.xlabel`, `plt.ylabel`, `plt.axis` and `plt.grid`, which target the
current axes rather than the supplied one. If the caller passes an
`ax` that is not the 'current' axes — e.g. one of several subplots in
a dashboard — the labels, aspect, and grid end up on the wrong panel.
Replace with `ax.set_xlabel`, `ax.set_ylabel`, `ax.set_aspect("equal")`
and `ax.grid`.
Written by Claude Code 4.6 Opus (high-effort).
The inline comment claimed the result was converted 'to microns', but the value is actually arcsec (pixels * arcsec/pixel). Also the sigma->FWHM factor was truncated to `2.355`; the exact value is `2*sqrt(2*ln(2)) = 2.3548...`. Replace the comment with one that states the conversion and units correctly and use `2.3548` so the numerical result rounds one decimal place closer to the truth. Written by Claude Code 4.6 Opus (high-effort).
The axis label is built up by replacing `u`->`Rx` and `v`->`Ry`, so the only way a rotation axis is flagged is by the uppercase `R` introduced by those replacements. The test `"r" in label` is case-sensitive and never matches uppercase `R`, so rotations were labelled with the translation unit `µm` instead of `deg`. Check for `"R"` instead so rotation axes get the correct degree unit. Written by Claude Code 4.6 Opus (high-effort).
The previous extremum-uncertainty expression was
`sqrt((2*vertex*da)^2 + db^2 + 4*vertex*covAB)`. That does not correspond
to any valid error-propagation derivation of the vertex value
`f(xv) = c - b^2/(4a)`:
* the `a`-term should be `vertex^4 * cov[a,a]`, not `4*vertex^2 * cov[a,a]`;
* the `b`-term should be `vertex^2 * cov[b,b]`, not `cov[b,b]`;
* `cov[c,c]`, `cov[a,c]`, and `cov[b,c]` were missing entirely;
* the `cov[a,b]` term should be `2*vertex^3 * cov[a,b]`, not
`4*vertex * cov[a,b]`;
* and because the `cov[a,b]` term entered unsquared, the argument of
the outer `sqrt` could go negative for well-behaved fits, producing
spurious NaN uncertainties.
Replace the formula with the correct expression derived from the partial
derivatives of `c - b^2/(4a)` evaluated at the vertex and drop the stale
TODO warning.
Written by Claude Code 4.6 Opus (high-effort).
The fit-failure fallback assigned `coeffs = (np.nan, np.nan, np.nan)` and the overlay was guarded by `if coeffs[0] is not np.nan:`. Identity comparison against `np.nan` is not a supported NaN test; every fresh NaN is a distinct object, so the guard evaluates to True and the code then plots a Gaussian curve with NaN parameters, producing warnings and no useful data. Use `not np.isnan(coeffs[0])`, which is the correct way to test for NaN. Written by Claude Code 4.6 Opus (high-effort).
The 'Skipping' branch printed a message but then fell through and still appended the image's fit data to ``fitData``, contradicting the printed message and allowing centroids that had drifted beyond ``maxDistance`` to poison the parabola fit. Add the missing ``continue`` so the loop really does skip these entries. Written by Claude Code 4.6 Opus (high-effort).
The previous commit added a ``continue`` so centroid outliers no longer
contribute to the fit, but ``fitData[seqNum] = {}`` is assigned earlier
in the same iteration. Without the paired ``del``, the skipped seqNum
remains in ``fitData`` as an empty dict and the subsequent
``fitDataAndPlot`` / ``getStarRadius`` helpers would ``KeyError`` on
``fitData[seqNum]["fitResult"]``. Remove the empty entry before
continuing.
Written by Claude Code 4.6 Opus (high-effort).
``fitDataAndPlot`` documents returning the best-focus hexapod position per spectrum slice, but the ``fitMin`` computation and the ``bestFits.append`` call lived inside the ``if not hideFit`` block, so passing ``hideFit=True`` returned an empty list. Pull the parabola minimum out of the conditional so only the plotting overlay is skipped and ``bestFits`` is always populated. Written by Claude Code 4.6 Opus (high-effort).
The Arrow overlay used variables named ``arrowy``/``arrowx`` to hold the x- and y-coordinates respectively, and then passed them through ``Arrow(arrowy, arrowx, dy, dx, ...)``. The two layers of swapping and the misleading comment (``# numpy is backwards``) cancel each other out so the arrow actually lands in the right place, but the code was actively confusing to read and very easy to 'fix' into a real bug. Rename to ``arrowX``/``arrowY``/``arrowDx``/``arrowDy`` and call ``Arrow`` with its natural signature; behaviour is unchanged. Written by Claude Code 4.6 Opus (high-effort).
The local ``arcminToPixel = 10`` constant was used to divide a Gaussian sigma (in pixels) to produce a width that was plotted with a y-label of 'FWHM (arcsec)'. The resulting value is indeed arcsec given the LATISS plate scale of ~0.1 arcsec/pixel (pixels * 0.1 = arcsec), so the numeric value is correct — but the variable name claimed the constant was an arcmin/pixel conversion. Rename to ``pixelsPerArcsec`` in both ``SpectralFocusAnalyzer.fitDataAndPlot`` and ``NonSpectralFocusAnalyzer.fitDataAndPlot`` so the name matches what the constant actually does. Written by Claude Code 4.6 Opus (high-effort).
All the other arguments to the per-record ``axvspan`` calls
(``startExposing``, ``endExposing``, and the annotation midpoint) are
Python ``datetime`` objects produced by ``.utc.datetime``, but
``readoutEnd`` was produced by ``.utc.to_value("isot")``, returning an
ISO-format string. Mixing a string with datetimes inside
``axvspan``/matplotlib's date-conversion path is fragile and can shift
the readout band depending on tz handling and matplotlib version. Use
``.utc.datetime`` so the argument type matches the other spans — this
is also the convention already used by the Simonyi variant.
Written by Claude Code 4.6 Opus (high-effort).
The ``plotHexapod`` branch fetched the ATAOS ``hexapodCorrectionStarted``/``hexapodCorrectionCompleted`` events using ``expRecord=record``, but ``record`` is the loop variable from an earlier ``for i, record in enumerate(expRecords)`` loop and by the time this branch runs it is bound to whatever the last iteration happened to leave behind. As a result the hexapod overlay only ever showed data for the final exposure's window instead of the whole plotted range. Fetch via ``begin``/``end`` instead, matching the way ``getCommands`` is already called above. Written by Claude Code 4.6 Opus (high-effort).
The ``if __name__ == '__main__'`` block imported ``plotExposureTiming`` from ``lsst.summit.extras.slewTiming``, which was renamed to ``slewTimingAuxTel`` (this module) and ``slewTimingSimonyi``. Running the file directly therefore raised ``ModuleNotFoundError``. ``plotExposureTiming`` is defined in this very module, so drop the re-import and let the example call the name directly. Written by Claude Code 4.6 Opus (high-effort).
The ``getAxisName`` helper classified topics by falling through a ladder of ``if any(x in topic for x in ...)`` checks. The ``'mount'`` branch listed ``MTM1M3`` and ``MTM2``, and the ``'aos'`` branch listed the same two names again. Because ``'mount'`` is checked first, the ``MTM1M3``/``MTM2`` entries in the AOS branch were dead code; a reader could easily misread this as 'some of these commands go to the AOS panel' when in fact all of them go to mount. Remove the duplicated entries from the ``'aos'`` branch without reordering (so existing behaviour is preserved); ``MTAOS``/``MTHexapod`` still route to ``'aos'``, and the ``m1m3Correction``/``m2Correction`` AOS events continue to match via their ``MTAOS`` prefix. Written by Claude Code 4.6 Opus (high-effort).
``plotExposureTiming`` called ``expRecords.sort(...)`` which mutates the caller's list in place — an unexpected side effect from a plot-making function. The AuxTel equivalent already uses ``sorted``. Rebind ``expRecords`` to a locally sorted copy so the caller's list order is preserved. Written by Claude Code 4.6 Opus (high-effort).
The Monitor's polling loop only called ``sleep(self.cadence)`` on the 'already displayed' early-continue branch. Both the normal display path and the ``NotFoundError`` branch fell through to the bottom of the ``for`` loop without any delay, so a successful display or a single ``NotFoundError`` caused the loop to immediately poll the butler again at full speed — a busy loop against the repo whenever a new image was unavailable. Move the ``sleep(self.cadence)`` out of the conditional so every iteration waits the configured cadence. Written by Claude Code 4.6 Opus (high-effort).
``ndarray.tostring`` was deprecated in NumPy 1.19 and removed in NumPy 2.0 in favour of ``ndarray.tobytes``. Calling it from ``_hashFile`` therefore raises ``AttributeError`` on any modern NumPy installation. Both methods return the raw byte representation of the array, so ``tobytes()`` is a drop-in replacement and produces the same hash inputs for matching dtypes. Written by Claude Code 4.6 Opus (high-effort).
The collision warning fired on any ``len(listOfKeys) != 1``, which
includes the zero-match case — and in that case the next line
(``return listOfKeys[0]``) raised ``IndexError`` instead of reporting
'no match'. The warning text was also misleading when only zero keys
were found ('Found multiple keys...').
Narrow the warning to the true collision case (``> 1``) and return
``None`` when no keys matched, so callers that pass
``returnCollisions=False`` can detect 'no match' without an exception.
Written by Claude Code 4.6 Opus (high-effort).
The two interactive confirmation prompts used ``if cont.lower()[0] != 'y':``. When the user just hits Enter, ``cont`` is the empty string and ``cont.lower()[0]`` raises ``IndexError``, leaking a traceback instead of exiting cleanly as the prompt clearly suggests. Treat an empty reply the same as any non-'y' reply — i.e. quit — by short-circuiting with ``not cont`` before indexing. Written by Claude Code 4.6 Opus (high-effort).
A helper named ``sorted`` was defined in this module that stringified its inputs (replacing blank FITS cards) before sorting. Once defined, every subsequent ``sorted(...)`` call in the module bound to this helper rather than the builtin, which was fragile: a reader reaching for ``from builtins import sorted as _sorted`` inside the helper is a strong smell, and any future code added further down the module would silently inherit the stringifying behaviour. Rename it to ``sortedAsStrings`` (and update all call sites in ``keyValuesSetFromFiles`` and ``compareHeaders``) so the builtin is visible again and the stringify-and-sort behaviour is opt-in. Written by Claude Code 4.6 Opus (high-effort).
``_runQFM`` did ``qfmRes = qfmResults.loc[i]`` and then wrote fields onto ``qfmRes``. ``DataFrame.loc[i]`` returns a Series that, for a mixed-dtype DataFrame, is a copy rather than a view — so the column assignments landed on the copy and the returned ``qfmResults`` kept the all-``NaN`` rows it was initialised with (the classic ``SettingWithCopy`` hazard). Replace the intermediate variable with ``qfmResults.at[i, <column>] = ...`` so the writes reliably land on the DataFrame that is eventually returned. Written by Claude Code 4.6 Opus (high-effort).
The failure-detection loop indexed directly into ``log[-1]``, which raises ``IndexError`` if the butler ever returns an empty log for a dataRef. Empty logs do occur in practice (e.g. for dataRefs where the quantum ran without emitting any log records). Guard the index with a ``len(log) == 0: continue`` so an empty log is simply treated as 'not failed' rather than crashing the whole browser. Written by Claude Code 4.6 Opus (high-effort).
``msg.split('Exception ')`` splits on every occurrence, so a log
message that mentions the word 'Exception ' in its own body — e.g.
a traceback chain along the lines of 'RuntimeError: Exception raised
while handling Exception ...' — produced more than two parts. The
code required ``len(parts) == 2`` / ``!= 2`` and therefore dropped
such failures from ``doFailZoology`` and fell back to the full message
in ``printFailLogs``/``printSingleLog``. Swap to ``str.partition``,
which splits only on the first occurrence and returns a
``(head, sep, tail)`` triple; test ``sep`` to detect whether the
delimiter was present. All three sites are updated together.
Written by Claude Code 4.6 Opus (high-effort).
The zoology count padding was computed as ``math.ceil(math.log10(maxCount))``, which gives the digit count only when ``maxCount`` is strictly greater than the nearest power of ten. For ``maxCount == 10`` it yielded ``1`` (needs ``2``), for ``maxCount == 100`` it yielded ``2`` (needs ``3``), etc., producing misaligned count columns whenever the largest count was exactly a power of ten. ``len(str(maxCount))`` is both correct and obviously reading the digit count. The ``math`` import is now unused, so drop it too. Written by Claude Code 4.6 Opus (high-effort).
``Logger.warn`` has been deprecated in favour of ``Logger.warning`` since Python 3.3 and is scheduled for removal. Switch the single call in ``LogBrowser.__init__`` to ``self.log.warning`` to avoid the deprecation warning and future breakage. Written by Claude Code 4.6 Opus (high-effort).
``isoparalacticAngle`` was implemented as ``return self.isoparalacticAngle``, which recurses forever and raises ``RecursionError`` whenever the property is accessed — i.e. the method simply did not work. There is also no corresponding field on ``SeeingConditions`` for it to forward to. Match the pattern used by ``groundLayerSeeing`` and raise a ``NotImplementedError`` with an explanatory message, and document the ``Raises`` section in the docstring, so callers get a clear failure instead of blowing the stack. Written by Claude Code 4.6 Opus (high-effort).
``starName`` was implemented as ``return f"HD{self.starName}"`` —
a property that recursively called itself, raising ``RecursionError``
on any access, and embedding 'HD' in front of another 'HD...' even
if it had worked. The RINGSS EFD topic supplies ``hrNum`` (the HR
catalogue number), which is the only star-identifier field stored on
``SeeingConditions``. Build the star name as ``f"HR{int(self.hrNum)}"``
(the HR catalogue prefix, not HD, because that is what the numeric
column actually refers to), and update the docstring example to
match.
Written by Claude Code 4.6 Opus (high-effort).
``SeeingConditions.__init__`` computed the interpolation point as ``t = t1 + (t2 - t1) / 2`` (the midpoint between the two source rows), then computed ``w = (t - t1) / (t2 - t1)``, which is 0.5 by construction. ``getSeeingAtTime`` also didn't pass the user's requested time at all. Net effect: 'interpolation' was always the simple mean of the two bracketing rows regardless of where the requested time fell. Thread an optional ``targetTime`` through ``__init__`` (keeping the midpoint fallback for callers that don't supply one), compute ``w`` against ``targetTime``, and have ``getSeeingAtTime`` pass the requested ``time`` so the interpolated row actually reflects that time. Clamp ``w`` to ``[0, 1]`` so any timestamp-ordering edge case doesn't extrapolate outside the bracketing rows. Written by Claude Code 4.6 Opus (high-effort).
``getSeeingAtTime`` mixed an ``astropy.time.Time`` with a pandas
``DatetimeIndex`` / ``Timestamp`` in several places:
* ``if time in data.index`` / ``data.loc[time]`` with a ``Time`` on
a tz-aware pandas ``DatetimeIndex`` does not match anything and
the exact-timestamp branch was dead.
* ``(time - earlier.name).sec`` mixed a ``Time`` and a pandas
``Timestamp``, which raises a ``TypeError`` when that branch
would actually fire (no ``later`` row).
* ``(laterTime - earlierTime).seconds`` uses ``Timedelta.seconds``,
which only returns the sub-day component (0-86399); intervals
longer than a day silently appear short.
Convert ``time`` to a tz-aware ``pd.Timestamp`` once up-front and
use it everywhere that compares against ``data.index``/``*.name``.
Use ``total_seconds()`` instead of ``.sec``/``.seconds`` so intervals
are reported correctly for any duration.
Written by Claude Code 4.6 Opus (high-effort).
``plotSeeing`` assigned ``df.index = pd.DatetimeIndex(...)`` but then
never used it: the three ``ax1.plot(...)`` calls were given only a
y-series, so matplotlib placed the points at x = 0, 1, 2, ... and the
subsequent ``DateFormatter('%H:%M:%S')`` + ``num2date`` magic on both
axes produced nonsense labels. The 'Time (UTC)' / 'Time (Chilean
Time)' x-axis captions therefore described empty-looking integer
tick values. Rebind the index into a local ``times`` variable and
pass it as the x-values of each plot so the time formatters and the
UTC/Santiago ticks line up with real timestamps.
Written by Claude Code 4.6 Opus (high-effort).
``1.1 * max([s.seeing2 for s in seeings])`` returns ``nan`` (or triggers ordering-dependent behaviour) the moment any ``seeing2`` value is NaN, and ``set_ylim(0, nan)`` raises/warns in matplotlib. Missing RINGSS measurements are common in practice, so NaNs in ``seeing2`` should not break the plot. Use ``np.nanmax`` (importing NumPy for this) so NaNs are ignored when setting the y-axis upper limit. Written by Claude Code 4.6 Opus (high-effort).
The metrics panel reported a value computed as ``np.sqrt(np.percentile(FWHM, 95) ** 2 - np.percentile(FWHM, 5) ** 2)`` but labelled it ``'sqrt(fwhm_95 - fwhm_05)'``, which is mathematically different. Update the label to include the ``^2`` exponents so the rendered string matches the computed quantity. Written by Claude Code 4.6 Opus (high-effort).
``groups['M1M3 bending modes']`` used ``np.arange(10, 30)``, which is 20 indices (10-29), but the comment described it as 'DOFs 17-29 (13 modes)'. Update the trailing comments on both bending-mode entries to state the actual DOF index ranges (10-29 and 30-49, 20 modes each) so future readers can't be misled by the mismatch, and drop the inaccurate/unnecessary range annotations on the decentering/tilt entries. Written by Claude Code 4.6 Opus (high-effort).
The predicted-FWHM panel overlaid two scatters (``fwhmWithAtm`` at source positions and ``cornersFwhmWithAtm`` at the corner-sensor field angles) but each used its own ``vmin/vmax = percentile(vals, [5, 95]) + median(dataset)``, and only the first scatter had an attached colorbar — so the colours of the corner dots did not correspond to the bar. The measured-FWHM panel next to it then used the same percentile offsets shifted by the measured median, so equal colours across the two panels did not represent equal absolute FWHM either. Compute one absolute ``fwhmVmin``/``fwhmVmax`` from the 5-95 percentile of the concatenation of all three data sets and apply it to every scatter in the AOS-FWHM and measured-FWHM panels. Now the shared colour bar is accurate for every point drawn and the two panels are directly comparable in absolute arcsec. Written by Claude Code 4.6 Opus (high-effort).
``os.path.join(self.pngPath, f'{uuid.uuid1()}/'[0:8])`` built the
UUID-suffixed directory by first appending a trailing ``'/'`` and then
slicing the first 8 characters of that. In practice the
``str(uuid.uuid1())`` form starts with 8 hex characters followed by
``-``, so the ``'/'[0:8]`` trick kept 8 hex characters and threw away
the appended slash, leaving a subdirectory name of only 8 hex chars.
That is harmlessly short when running once, but gives just 2^32
possibilities for concurrent runs and relies on a very opaque
expression. Replace with ``uuid.uuid1().hex[:8]``, which is
self-documenting and produces the same character-count directory
name.
Written by Claude Code 4.6 Opus (high-effort).
``assert False, '...'`` is skipped entirely when Python is run with ``-O``, which would turn an invariant violation (about to overwrite an existing PNG without ``remakePngs=True``) into silent wrong behaviour. Raise ``RuntimeError`` with the same message so the check is preserved regardless of optimisation flags. Written by Claude Code 4.6 Opus (high-effort).
The ``exactMatches=True`` path combined two conditions: ``all(t in tag for t in tags.upper())`` (superset check) and ``len(tags) == len(tag)`` (length check). For queries with a duplicate character — e.g. ``tags='ff'`` — the superset check is satisfied by *any* tag containing ``F``, and the length check is satisfied by any 2-character tag. So ``'ff'`` would match ``'FD'``, which is clearly wrong. Build ``wanted = set(tags.upper())`` once and return rows where the stored tag's character set equals ``wanted``. This gives true 'exact match' semantics (order- and duplicate-insensitive) without false-positive matches from duplicated query characters. Written by Claude Code 4.6 Opus (high-effort).
CLAUDE.md asks that ``__init__.py`` be kept in sync with the public classes the package exposes. Since ``focusAnalysis``, ``animation``, ``headerFunctions``, ``imageSorter`` and ``monitoring`` were added years ago several modules with well-known public entry points never made it into the top-level namespace: * ``annotations.Annotations`` * ``assessQFM.AssessQFM`` * ``fastStarTrackerAnalysis`` (via its ``__all__``) * ``logUtils.LogBrowser`` * ``ringssSeeing.SeeingConditions`` / ``RingssSeeingMonitor`` Wildcard-import modules that define ``__all__`` (``logUtils``, ``fastStarTrackerAnalysis``) and name-import the classes from modules that do not, so notebook users can ``from lsst.summit.extras import ...`` without caring which submodule a class lives in. ``slewTimingAuxTel`` and ``slewTimingSimonyi`` both define ``plotExposureTiming`` and would collide under ``import *``; leave them qualified with an inline comment explaining why. Written by Claude Code 4.6 Opus (high-effort).
A newly-added explanatory comment in ``getSeeingAtTime`` was 80 characters wide, tripping the project's flake8 ``W505`` (doc-line length 79). Rephrase slightly to come under the limit. Written by Claude Code 4.6 Opus (high-effort).
The preceding commits introduced two list comprehensions/plot calls that happened to fit on a single 110-char line but had been broken onto three lines during editing. Running the project's configured ``black`` (line-length 110) over the touched files collapses them back onto a single line to match the rest of the codebase. Written by Claude Code 4.6 Opus (high-effort).
mfisherlevine
force-pushed
the
tickets/DM-51705
branch
from
f717d10 to
40c0d03
Compare
So that `mypy .` doesn't trip over the duplicate modules between bin.src/ (source) and bin/ (SCons-generated copies). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Calls the shared lsst/rubin_workflows mypy workflow (the same pattern already used here for lint, formatting, and rebase_checker, and the same pattern summit_utils uses for its mypy gate). Adds a requirements.txt listing the pip-installable deps mypy needs to resolve imports the C++-heavy LSST packages aren't available on PyPI but are already covered by ignore_missing_imports in mypy.ini. Branch protection still needs to be updated separately to make this a required check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mypy-coverage report❌ 18 unannotated, 8 partial definition(s).
Summary
Files with gaps
Unannotated definitions (18)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.