Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-34105: Move some generic code from obs_base #239

Merged
merged 11 commits into from Mar 23, 2022
Merged

DM-34105: Move some generic code from obs_base #239

merged 11 commits into from Mar 23, 2022

Conversation

timj
Copy link
Member

@timj timj commented Mar 21, 2022

obs_base is going to be hard to disentangle from afw, ip_isr, and pipe_tasks. Move a subset of Instrument class to pipe_base along with support material.

Checklist

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

Black makes them irrelevant.
@codecov
Copy link

codecov bot commented Mar 22, 2022

Codecov Report

Merging #239 (bd34f33) into main (2672429) will increase coverage by 0.62%.
The diff coverage is 86.98%.

❗ Current head bd34f33 differs from pull request most recent head 11741c1. Consider uploading reports for the commit 11741c1 to get more accurate results

@@            Coverage Diff             @@
##             main     #239      +/-   ##
==========================================
+ Coverage   71.55%   72.17%   +0.62%     
==========================================
  Files          48       60      +12     
  Lines        6313     6595     +282     
  Branches     1216     1244      +28     
==========================================
+ Hits         4517     4760     +243     
- Misses       1574     1606      +32     
- Partials      222      229       +7     
Impacted Files Coverage Δ
...ython/lsst/pipe/base/script/register_instrument.py 55.55% <55.55%> (ø)
python/lsst/pipe/base/_instrument.py 75.49% <75.49%> (ø)
python/lsst/pipe/base/tests/simpleQGraph.py 67.54% <85.71%> (-1.53%) ⬇️
python/lsst/pipe/base/formatters/pexConfig.py 87.50% <87.50%> (ø)
tests/test_cliCmdRegisterInstrument.py 89.47% <89.47%> (ø)
tests/test_config_formatter.py 92.85% <92.85%> (ø)
tests/test_instrument.py 97.70% <97.70%> (ø)
python/lsst/pipe/base/__init__.py 100.00% <100.00%> (ø)
python/lsst/pipe/base/cli/cmd/__init__.py 100.00% <100.00%> (ø)
python/lsst/pipe/base/cli/cmd/commands.py 100.00% <100.00%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2672429...11741c1. Read the comment docs.

@timj timj force-pushed the tickets/DM-34105 branch 3 times, most recently from 0f4e6c8 to 137e070 Compare March 22, 2022 21:36
Copy link
Contributor

@andy-slac andy-slac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, few comments about docstrings/comments.

python/lsst/pipe/base/_instrument.py Outdated Show resolved Hide resolved
python/lsst/pipe/base/formatters/pexConfig.py Outdated Show resolved Hide resolved
python/lsst/pipe/base/script/register_instrument.py Outdated Show resolved Hide resolved
tests/test_config_formatter.py Outdated Show resolved Hide resolved
This is the obs_base Instrument class with the afw
dependencies and the curated calibrations removed.
The curated calibrations core functionality may well
come back but needs some thought.
No longer need to fake one since the class does exist
now in pipe_base.
This includes setting up of some click infrastructure
since this is the first click command in pipe_base
There seems to be a strange interaction with the click
command line test messing with the logger if called
before this test and it doesn't hurt to be more specific.
Packages has moved from base to utils and can now be assumed
to always be available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants