Enable CRREJ on acs_destripe_plus products

[pllim]

Fix #35 -- Enable CRREJ on acs_destripe_plus products. Needed to add a new function named crrej_plus() due to the recursive nature of acs_destripe_plus(), which was meant to process individual exposures, not combine them.

Fix #66

Bonus: Removed unexpected _RULES_ section warning when using TEAL interface. Some minor PEP 8 clean-ups on the module.

c/c @jryon , @nmiles2718 , and @tddesjardins

[dborncamp]

This should do what I wanted. Thanks @pllim !

[pllim]

That's great news, @dborncamp ! I hope one of the people on ACS Team can take this for a spin when they get the chance later. Thanks for the review.

[pllim]

@jryon , do you also want an option to keep temp files here, or not? Please advise. Thanks!

[jryon]

It looks like these temp files are just renamed BLV_TMP/BLC_TMP files. Since crrej_plus needs BLV_TMP/BLC_TMP files as input, these should already be available, right?

[pllim]

I don't remember why I added this now, maybe to be consistent with original behavior... Should I just delete the "delete intermediate file" logic here, or you want an option to toggle it?

[jryon]

Oh, I misread this before. I think an option to toggle keeping the CR-rejected BLVs around could be helpful. I like having the option to see the result of every step in the typical calibration process if desired.

[pllim]

OK done @jryon

[nmiles2718]

@pllim the keep_intermediate_files flag seems to have no affect. I ran this on a test set of subarrays, ensured that the acs_destripe_plus version matches that quoted on L102 (v 0.4.3), set keep_intermediate_files=True but the *blv_tmp.fits were still deleted.

In [1]: from acstools import acs_destripe_plus
The following tasks in the acstools package can be run with TEAL:
          acs2d                  acs_destripe           acs_destripe_plus     
          acsccd                    acscte              acscteforwardmodel    
          acsrej                    acssum                    calacs
PixCteCorr is no longer supported. Please use acscte.

In [2]: acs_destripe_plus.__version__
Out[2]: '0.4.3'

In [3]: acs_destripe_plus.destripe_plus('*raw.fits',cte_correct=False, keep_intermediate_files=True)
git tag: 2.0.0
git branch: HEAD
HEAD @: ce7fe09f255b882f816cc3c39d0102f58689f76f
Trying to open jd5702kcq_raw.fits...
Read in Primary header from jd5702kcq_raw.fits...
.
. OMITTING CALACS OUTPUT FOR BREVITY
.
INFO:acs_destripe_plus:Done.
FLT: jd5702kcq_flt.fits

git tag: 2.0.0
git branch: HEAD
HEAD @: ce7fe09f255b882f816cc3c39d0102f58689f76f
Trying to open jd5702kdq_raw.fits...
Read in Primary header from jd5702kdq_raw.fits...
.
. OMITTING CALACS OUTPUT FOR BREVITY
.
*** ACS2D complete ***
INFO:acs_destripe_plus:Done.
FLT: jd5702kdq_flt.fits


In [4]: ls
CODE_OF_CONDUCT.md  RELIC-INFO          dist/               jd5702kcq_raw.fits  jd5702kdq_raw.fits  setup.cfg
LICENSE.md          acstools/           doc/                jd5702kcq_spt.fits  jd5702kdq_spt.fits  setup.py*
MANIFEST.in         acstools.egg-info/  jd5702kcq.tra       jd5702kdq.tra       n4e12511j_crr.fits  test/
README.rst          build/              jd5702kcq_flt.fits  jd5702kdq_flt.fits  relic/

Note that the expected blv_tmp files are not present. I took a look at the code and logic is fine. My guess is that they're probably being deleted by acs2d and that you'll have to extend the the logic to that call as well. I tried passing exe_args=['-s'] in L501 but this resulted in an error saying that -s is an unrecognized option for acs2d.

Unrecognized option -s
syntax:  acs2d [--help] [-t] [-v] [-q] [--version] [--gitinfo] input output
---------------------------------------------------------------------------
CalledProcessError                        Traceback (most recent call last)
<ipython-input-2-a7c7340c7bfb> in <module>()
----> 1 acs_destripe_plus.destripe_plus('*raw.fits',cte_correct=False,keep_intermediate_files=True)

/user/nmiles/acs_destripe_test/acstools/acs_destripe_plus.py in destripe_plus(inputfile, suffix, stat, maxiter, sigrej, lower, upper, binwidth, scimask1, scimask2, dqbits, rpt_clean, atol, cte_correct, keep_intermediate_files, clobber, verbose)
    365                 maxiter=maxiter, sigrej=sigrej,
    366                 scimask1=scimask1, scimask2=scimask2, dqbits=dqbits,
--> 367                 cte_correct=cte_correct, clobber=clobber, verbose=verbose
    368             )
    369         return

/user/nmiles/acs_destripe_test/acstools/acs_destripe_plus.py in destripe_plus(inputfile, suffix, stat, maxiter, sigrej, lower, upper, binwidth, scimask1, scimask2, dqbits, rpt_clean, atol, cte_correct, keep_intermediate_files, clobber, verbose)
    499 
    500     # run ACS2D to get FLT and FLC images
--> 501     acs2d.acs2d(blvtmp_name,exe_args=['-s'])
    502     if cte_correct:
    503         acs2d.acs2d(blctmp_name)

/user/nmiles/acs_destripe_test/acstools/acs2d.py in acs2d(input, exec_path, time_stamps, verbose, quiet, exe_args)
    111         call_list.extend(exe_args)
    112 
--> 113     subprocess.check_call(call_list)
    114 
    115 

~/miniconda3/envs/destripe_plus_updates/lib/python3.6/subprocess.py in check_call(*popenargs, **kwargs)
    289         if cmd is None:
    290             cmd = popenargs[0]
--> 291         raise CalledProcessError(retcode, cmd)
    292     return 0
    293 

CalledProcessError: Command '['acs2d.e', 'jd5702kcq_blv_tmp.fits', '-s']' returned non-zero exit status 1.

[pllim]

Thanks for checking! I agree with your assessment. I do not wish to mess with HSTCAL in the midst of 2018.3, so I'll have to think about this... 🤔

[pllim]

This is now stale. Let's re-visit at #66 (comment) and go from there.

[pllim]

Re: #46 (comment)

I am not convinced that acs2d.e removes the input file anymore. I cannot reproduce the reported behavior using acs2d.e nor acstools.acs2d.acs2d().