DM-15189: Initial implementation of Gen3 raw-data ingest #107
Conversation
python/lsst/obs/base/gen3/ingest.py
Outdated
| config : `RawIngestConfig` | ||
| Configuration for whether/how to transfer files and how to handle | ||
| conflicts and errors. | ||
| butler : `daf.butler.Butler` |
There was a problem hiding this comment.
All your class references in the docstrings need the lsst. in front. Consider using ~lsst.daf.butler.Butler etc.
python/lsst/obs/base/gen3/ingest.py
Outdated
| try: | ||
| self.processFile(os.path.abspath(file)) | ||
| except Exception as err: | ||
| self.log.warn("Error processing '{}': {}".format(file, err)) |
There was a problem hiding this comment.
Use:
self.log.warnf("Error processing '{}': {}", file, err)to defer the formatting until you know the message will appear.
python/lsst/obs/base/gen3/ingest.py
Outdated
| raise IngestConflictError("Ingest conflict on {} {}".format(file, dataId)) | ||
|
|
||
| # Add component Dataset entries, assuming this is a concrete | ||
| # composite. We may need to revisit that assumption in the future. |
There was a problem hiding this comment.
If raw always means "entity written by the telescope for this observation" then isn't raw by definition always a concrete composite?
There was a problem hiding this comment.
I hope so. I'm worried about TransmissionCurves and Detectors, which are Exposure components that we keep in calibration repositories; if we can defer attaching those to Exposures until ISR or some pre-ISR PipelineTask, we're fine. But that makes it very inconvenient for people doing ad-hoc things with raw data, as lots of people will be doing in commissioning.
python/lsst/obs/base/gen3/ingest.py
Outdated
| # clause above; if we get a conflict at this point the Registry is | ||
| # corrupted somehow, and so we want that more serious error to | ||
| # propagate up to the *onError* handling in run(). | ||
| for component in ref.datasetType.storageClass.components: |
There was a problem hiding this comment.
The code duplication here from Butler.put() is a little annoying.
There was a problem hiding this comment.
Agreed. What do you think about a recursive boolean keyword argument to Registry.addDataset that would run this logic there?
TallJimbo
force-pushed
the
tickets/DM-15189
branch
from
2625966
to
f790e0d
Compare
TallJimbo
force-pushed
the
tickets/DM-15189
branch
from
f790e0d
to
bddf8d9
Compare
|
Jim and I've been discussing access to raw data. It is very convenient to be able to say |
No description provided.