DM-36211: Add PeekExposureTask

[jmeyers314]

Adds PeekExposureTask as a solution to quickly find the brightest source (for observing scripts), and also quickly estimate PSF moments along xy, altaz, and equitorial axes.

[jmeyers314]

Started addressing flake8 failures reported here, then noticed that at some of my original formatting was apparently set by running first isort then black. Am I wrong to think that black ought to produce acceptable whitespace-around-: and linebreak-before-binary-operator rules? (Some of the other flake8 complaints are definitely legitimate though...)

[jmeyers314]

Looks like the setup.cfg could set W503 under ignore as documented here? And E203 is mentioned here

Though I'm unsure if lsst-sitcom doesn't follow the same dev guide as lsst maybe?

[mfisherlevine]

It does (currently, intend to) follow the same dev guide, yes. But it's not black and isorted (yet), and there are places where black and flake8 are mutually exclusive, in that black will autoformat things into a state that is not compatible with having an empty flake8 ignore-list, and the ignores picked by, for example, middleware packages, are not the same as those in pipelines, which is what this package follows. It's all a bit of a mess, but yes, the tl;dr is that black will, with the current configs, fight with flake8.

[mfisherlevine]

The easiest thing to do is to fixup those problems by hand for now, sorry. In the future, once I've merged some large tickets, we'll black and isort everything, but I need to merge some monster tickets because otherwise I'll be rebase hell.

[jmeyers314]

Well, easiest may be to just add W503 and E203 to https://github.com/lsst-sitcom/summit_utils/blob/main/setup.cfg#L4 (If I'm reading the dev guide right, then I think W503 should be added regardless of any potential black transition). But I'm happy to do either.

[TallJimbo]

I'm hitting Approve because I don't know what the standards are for this package, or more importantly where the balance is between standing something up quickly and being able to maintain it, and so I'm happy to defer to the author on that. There are issues here that I'd consider a maintenance problem if the code was going into lsst_distrib.

[TallJimbo]

Is this a TODO entry that was suppose to happen on this branch?

[jmeyers314]

More of: started working on it, decided I didn't need it, but didn't want to lose the partial progress. So my tentative plan was to commit it, and then delete it so it at least lives on in git history.

[mfisherlevine]

This and initializer() both need docstrings, especially as this is some of the weirder bits of the code. Also, I'd rename them both tbh: initializer sounds like a class, the function would more likely be verb, and doit() could at least be called doWork do runTask or something?

[mfisherlevine]

Oh, is this the only reason nobin exists? If so then I think it's fine to leave it as-is, and just have the binning controlled by the binning factor then (as people can just use 1 for no binning). Out of interest though, why is this necessarily not binned? Because in this case we're known to be struggling, and therefore our best chance of winning is to go totally unbinned?

[jmeyers314]

Oh, I think I remember now. It's because having passed through self.spec.run, the exposure is already binned, so this is to prevent binning it a second time...

[jmeyers314]

Although, the clone() was supposed to prevent that? I'll have to trace through to be certain...

[mfisherlevine]

Lots of quite picky comments from me, sorry. Generally, this package follows the DM dev guide as closely as is reasonably possible (and it's generally possible to follow it completely 🙂).

[mfisherlevine]

Also, and this one is really annoying, but this package is in camelCase, and this is bringing in a mix of snake and camel. Hopefully a quick find & replace can fix that up and not be too onerous?! 😬 Sorry!