DM-8798: Ingest HiTS calibration products and test on raw images #1
Conversation
…te3.IntegrityError
hits_ingest.py
Outdated
| # images and calibration products | ||
| if doProcessCcd: | ||
| print('Running ProcessCcd...') | ||
| OBS_DECAM_DIR = os.getenv('OBS_DECAM_DIR') |
There was a problem hiding this comment.
from lsst.utils import getPackageDir
getPackageDir("obs_decam")
hits_ingest.py
Outdated
| if doProcessCcd: | ||
| print('Running ProcessCcd...') | ||
| OBS_DECAM_DIR = os.getenv('OBS_DECAM_DIR') | ||
| os.chdir(calibrepo) |
There was a problem hiding this comment.
I worry a little whenever a script needs to chdir; is this a consequence of DM-5466? If so, is it possible to ingest the calibs with absolute paths instead of relative, so this isn't required?
There was a problem hiding this comment.
As far as I can tell, this is a consequence of DM-5466, which I really do plan to tackle soon. I don't like having the chdir in here either so I will add a comment/note to revisit this once obs_decam is a little smarter about calib paths.
| print('lsst.obs.decam.DecamMapper', file=f) | ||
| f.close() | ||
|
|
||
| if doIngest == True: | ||
| if doIngest: |
There was a problem hiding this comment.
This might just be a stylistic opinion on my part, but I think it would be a little nicer if the three different stages each lived in their own functions. Provides a bit more isolation (particularly when you reuse names like args and config) and visual definition.
There was a problem hiding this comment.
I agree; I plan to implement this going forward, but not for this ticket.
There was a problem hiding this comment.
Right, I forgot to mention that I'm totally on board with some of these being "out of scope" for this ticket. No worries
hits_ingest.py
Outdated
| elif not doIngest and not doIngestCalibs and not doProcessCcd: | ||
| raise RuntimeError('Nothing to do, every task is set to False') | ||
|
|
||
| try: |
There was a problem hiding this comment.
This should just be an if statement (if len(sys.argv) < 2:). Or argparse could handle this for you.
|
|
||
| # first, need to make a text file that handles the mapper, per the obs_decam github README | ||
| f = open(repodir+'_mapper', 'w') | ||
| # edit values below as desired |
There was a problem hiding this comment.
I think this block below here could basically be replaced with argparse and would be a bit more functional. Plus, you could make the --help text print the big message you have above, which would be very handy.
There was a problem hiding this comment.
I'm not very familiar with argparse, but I will find some examples and keep this in mind going forward.
hits_ingest.py
Outdated
| print('WARNING: no data directory specified.') | ||
| print('(This is OK if you are just running ProcessCcd.)') | ||
| else: | ||
| if datadir[-1] == '/': |
There was a problem hiding this comment.
Instead of relying on a /, which I would invariably forget had meaning, it might be nicer to use something like os.path.isdir().
Note: some of these commits have different usernames because git knew me as "mrawls" on lsst-dev and "Meredith Rawls" on my laptop. I have fixed it so both places use my full name.