DM-41606: Option to output pipetask report info to command line #277

eigerx · 2023-12-06T21:53:45Z

Checklist

ran Jenkins
added a release note for user-visible changes to doc/changes

codecov · 2023-12-06T21:57:57Z

Codecov Report

Attention: Patch coverage is 93.02326% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 87.44%. Comparing base (1ce94f1) to head (39d2baa).

Files	Patch %	Lines
python/lsst/ctrl/mpexec/cli/script/report.py	90.90%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #277      +/-   ##
==========================================
+ Coverage   87.40%   87.44%   +0.03%     
==========================================
  Files          49       49              
  Lines        4431     4469      +38     
  Branches      764      771       +7     
==========================================
+ Hits         3873     3908      +35     
- Misses        406      408       +2     
- Partials      152      153       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

eigerx · 2024-02-22T19:27:16Z

It looks like the mypy failure is due to ctrl_mpexec not knowing about the added option (human-readable) in the corresponding branch of pipe_base. This passes on Jenkins: https://rubin-ci.slac.stanford.edu/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/739/pipeline

timj · 2024-02-22T20:07:32Z

@eigerx fix mypy by temporarily editing requirements.txt to use the ticket branch rather than main.

timj · 2024-02-23T20:50:53Z

requirements.txt

 git+https://github.com/lsst/pex_config@main#egg=lsst-pex-config
 sqlalchemy
+types-psutil
+types-psycopg2


FYI types go in a file called types.txt and not in requirements.txt. The mypy action looks in a special place because they aren't real requirements.

I'm a bit confused about the psutil one since psutil is only used by test code and so shouldn't get involved in mypy (it also is in the wrong place in pyproject.toml -- looks like it's been in the wrong place since the beginning so not related to this PR). Also, psycopg2 is not used anywhere in ctrl_mpexec so maybe I need to look at the mypy error.

AH, ok. I will move these edits to types.txt. I googled the mypy error and the first suggestion I tried was mypy --install-types and those were what it installed. They didn't end up resulting in a "pass" from mypy here, though. The other suggested line, python3 -m pip install types-PyYAML, did work, and that was what installed types-PyYAML. So, I'll do a hard reset and move that line to types.txt. Thank you for the help!!

@timj it looks like that worked! Thanks for the linting help :)

kherner

The new code itself generally looks OK; I would guess it wouldn't be that hard to figure out the testing complaints with a few additions (i.e. a lot of it could probably be fixed with one or two calls). We can talk about that next week though.

kherner · 2024-03-03T05:41:15Z

requirements.txt

@@ -6,6 +6,6 @@ networkx
 git+https://github.com/lsst/resources@main#egg=lsst-resources
 git+https://github.com/lsst/daf_butler@main#egg=lsst-daf-butler
 git+https://github.com/lsst/utils@main#egg=lsst-utils
-git+https://github.com/lsst/pipe_base@main#egg=lsst-pipe-base
+git+https://github.com/lsst/pipe_base@tickets/DM-41606#egg=lsst-pipe-base


Also here don't forget to change it back when merging.

There is a GitHub action that prevents merging unless this has been removed.

kherner · 2024-03-03T05:42:54Z

python/lsst/ctrl/mpexec/cli/script/report.py

-    --------
-    lsst.pipe.base.QuantumGraphExecutionReport.make_reports : Making reports.
-    lsst.pipe.base.QuantumGraphExecutionReport.write_summary_yaml : Summaries.
+        butler_config : `str`


The indenting looks a little funny here relative to the old doc string, like it's an extra 4 spaces over.

Agreed. I'm kind of surprised that the numpydoc validator doesn't spot this -- maybe I've missed some configuration setting for numpydoc. velin (which you have to install from pypi) does notice and fix it.

kherner · 2024-03-03T05:43:03Z

python/lsst/ctrl/mpexec/cli/script/report.py


+import yaml


Switch the blank line order here. See PEP8 for import format.

We use isort and black for formatting so I think this is right. pprint is part of core python and yaml is not so a line separates them.

kherner · 2024-03-03T05:44:01Z

python/lsst/ctrl/mpexec/cli/script/report.py


+import yaml
+from astropy.table import Table
 from lsst.daf.butler import Butler


Probably should be a blank line between the astropy and Butler imports too, if you're being a format stickler.

isort won't allow that.

kherner

The changes to increase the test coverage look good.

eigerx force-pushed the tickets/DM-41606 branch 4 times, most recently from 7b3e706 to 3cc19ee Compare January 23, 2024 02:31

eigerx force-pushed the tickets/DM-41606 branch from 3cc19ee to e1fb02a Compare February 21, 2024 20:33

timj reviewed Feb 23, 2024

View reviewed changes

eigerx force-pushed the tickets/DM-41606 branch from 0f7e569 to 3a97ffd Compare February 23, 2024 22:17

kherner requested changes Mar 3, 2024

View reviewed changes

kherner approved these changes Mar 13, 2024

View reviewed changes

eigerx force-pushed the tickets/DM-41606 branch from 89906b7 to 317396f Compare March 14, 2024 00:12

eigerx added 2 commits March 13, 2024 17:13

Print table of produced and missing datasets to the command-line

7e9983a

Print table of failed and blocked quanta per task to the command-line

39d2baa

eigerx force-pushed the tickets/DM-41606 branch from 317396f to 39d2baa Compare March 14, 2024 00:14

eigerx merged commit a2f6597 into main Mar 14, 2024
15 checks passed

eigerx deleted the tickets/DM-41606 branch March 14, 2024 00:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DM-41606: Option to output pipetask report info to command line #277

DM-41606: Option to output pipetask report info to command line #277

eigerx commented Dec 6, 2023 •

edited

Loading

codecov bot commented Dec 6, 2023 •

edited

Loading

eigerx commented Feb 22, 2024

timj commented Feb 22, 2024

timj Feb 23, 2024 •

edited

Loading

eigerx Feb 23, 2024

eigerx Feb 23, 2024

kherner left a comment

kherner Mar 3, 2024

timj Mar 3, 2024

kherner Mar 3, 2024

timj Mar 3, 2024

kherner Mar 3, 2024

timj Mar 3, 2024

kherner Mar 3, 2024

timj Mar 3, 2024

kherner left a comment

DM-41606: Option to output pipetask report info to command line #277

DM-41606: Option to output pipetask report info to command line #277

Conversation

eigerx commented Dec 6, 2023 • edited Loading

Checklist

codecov bot commented Dec 6, 2023 • edited Loading

Codecov Report

eigerx commented Feb 22, 2024

timj commented Feb 22, 2024

timj Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kherner left a 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

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

kherner left a comment

Choose a reason for hiding this comment

eigerx commented Dec 6, 2023 •

edited

Loading

codecov bot commented Dec 6, 2023 •

edited

Loading

timj Feb 23, 2024 •

edited

Loading