Adding source detection step

[cshanahan1]

Resolves RCAL-423

Adds the source catalog step, which uses DAOStarFinder to detect point sources and writes out and returns a catalog.

Checklist

• added entry in CHANGES.rst under the corresponding subsection
• updated relevant tests
• updated relevant documentation
• updated relevant milestone(s)
• added relevant label(s)

[nden]

@cshanahan1 Please fix style changes and rebase

[schlafly]

This looks reasonable to me. It looks like the style failure has to do with some kind of configuration error with the new pre-commit step? And then we need to add photutils as a dependency so that the tests can run?

[codecov]

Codecov Report

Patch coverage: 72.72% and project coverage change: +0.17 🎉

Comparison is base (6cac937) 63.13% compared to head (66c508c) 63.31%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #608      +/-   ##
==========================================
+ Coverage   63.13%   63.31%   +0.17%     
==========================================
  Files          76       78       +2     
  Lines        4036     4113      +77     
==========================================
+ Hits         2548     2604      +56     
- Misses       1488     1509      +21     

Flag	Coverage Δ		*Carryforward flag
nightly	63.07% <ø> (ø)		Carriedforward from 3b312cb

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files	Coverage Δ
romancal/lib/suffix.py	94.28% <ø> (ø)
romancal/pipeline/exposure_pipeline.py	88.88% <50.00%> (-1.12%)	⬇️
romancal/source_detection/source_detection_step.py	72.60% <72.60%> (ø)
romancal/source_detection/__init__.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[mairanteodoro]

Looks good to me.

[schlafly]

This looks good to me. I added some comments in-line that may resolve the half-pixel offsets you're seeing. Most important outstanding questions for me are:

• Do you still think there are especially fragile bits of source detection that we should be tracking? I think I'd probably record them with comments.
• I had the impression in our testing that we found that subtracting the background and then photometering the sky-subtracted image was better performing than including the background in the threshold. Has your view there developed?

[cshanahan1]

is this ready to merge?

[mairanteodoro]

Should we merge RAD PR#215 and roman_datamodels PR#132 first?

[cshanahan1]

The only checks here failing are codecov. I did write tests that should cover every line, but they are not enabled under CI because they require a connection to roman CRDS. Is there a way to run the step and not require this connection so the tests can run? this step doesnt require a reference file.

[mairanteodoro]

The only checks here failing are codecov. I did write tests that should cover every line, but they are not enabled under CI because they require a connection to roman CRDS. Is there a way to run the step and not require this connection so the tests can run? this step doesnt require a reference file.

I think that if the test doesn't need a reference file then it should be safe to remove theskipif decorator.

[cshanahan1]

The only checks here failing are codecov. I did write tests that should cover every line, but they are not enabled under CI because they require a connection to roman CRDS. Is there a way to run the step and not require this connection so the tests can run? this step doesnt require a reference file.

I think that if the test doesn't need a reference file then it should be safe to remove theskipif decorator.

I tried skipping the decorator but it still tries to make the connection

[mairanteodoro]

If the tests pass, I'm good with merging this PR.

[cshanahan1]

i'm going to merge this once checks pass