Al-522: Adding a DQ flags parameter to ramp_fit to make them pipeline indepedent

[kmacdonald-stsci]

The DQ flags were originally defined in the JWST pipeline and defined globally. The move to STCAL for ramp fitting to be used by multiple pipelines, there is a possibility the flags could be different. To accommodate for this, DQ flags are now passed as a parameter to ramp_fit as a dictionary with five required keywords.

[nden]

Looks good. A couple minor comments.

[nden]

You could do constants.dqflags.update(dqflags) to update the entire dict at once.
We probably need to make sure all flags have a valid value

if None in constants.dqflags.values():
   raise ValueError

[kmacdonald-stsci]

I created an update function and a NoneType check.

[nden]

Start a new section in the Changes.rst and move the log there.

0.2.2 (Unreleased)
==================

ramp_fitting
------------

[kmacdonald-stsci]

Made these changes to the change log.

[dmggh]

Looks good to me. Eventually both mission's ramp_fit_step.py will also need to be modified for this new parameter. I don't recall what other changes are also needed for parameter files.

[kmacdonald-stsci]

I opened a PR on JWST changing the unit tests and step code to handle this additional parameter: spacetelescope/jwst#6072

[jdavies-st]

I realize this review is a bit late, as now this is merged. But here it is anyway.

[jdavies-st]

If dqflags is a required parameter to pass to ramp_fit now, what's the purpose of having flags defined as constants in a separate module? Shouldn't all the flag mnemonics be happening in ramp_fit and then passed along directly to any function it calls?

Having these defined in a module constants also implies they are constants, which clearly they no longer are if they can be changed.

[jdavies-st]

jump_flag is a pointer to the mnemonic `dqflags["JUMP_DET"], but here you use the mnemonic itself. Consistency would be good. I.e. either always use long-form mnemonics directly from the dict, or always use a variable, preferably defined at top of function.

[jdavies-st]

It looks like constants are no longer constants. ;-)

This is sort-of-a round-about way of getting the dict passed to the calling code might be better achieved by just passing the dict directly to ols_ramp_fit. The indirection here is a bit confusing.

Clearly this is a dict that needs to be shared by a number a functions, so a class structure would be better encapsulate what is going on here. But that may be too much of a leap for this PR.

[kmacdonald-stsci]

That is the long term goal. In JP-553 I am creating an internal class. The needed flags are there. I will eventually remove this global variable. Not only are they not constants anymore, using global variables that can change can have bad side effects. This PR will be the last that requires changes to the JWST code. The changes regarding the internal class and the proper passing around of all the needed information, such as the DQ flags will not effect JWST anymore, i.e., a mirror PR in JWST will not be needed when a PR is opened in STCAL ramp fitting.