Hey Matthew, this looks like a good start. A couple of notes:

1. In the first assert statement, you missed a .meta after ccddata.
2. I don't really like the optional flag. That requires you to end up doing some transformations to check on it whereas I much prefer the explicit keywords. Also the keywords could have important information for their values and not just that they were done.
3. The keywords should be FITS compliant, so that they can be easily written out. That means limited to 8 characters in length.

I'm also thinking about a third option of including something like:

def subtract_bias(ccddata, masterbias, add_keyword='SUBBIAS')

Users or groups may want to specify a keyword for internal purposes

I took out the flag option and put in your user-supplied keyword. That seems like the most flexible of the options -- one thought is to modify the Keyword class a bit to allow it to have a string value so that the user can either do

subtract_bias(ccddata, masterbias, add_keyword='SUBBIAS')

or

subtract_bias(ccddata, masterbias, add_keyword=a_keyword_object_with_name_and_value)

Regarding FITS compliance, I'd lean towards letting astropy.io.fits handle that on writing the file.

If you add a long keyword to a astropy.io.fits.Header you get a warning that it is creating a HIERARCH card, but it still handles the keyword.

One thing nice about your logging suggestion is that if there are users that want to avoid HIERARCH they have the option of choosing to add only keywords that are 8 characters or less

If we are dictating the keyword then I'd lean towards having users opt-in to a strict 8 character system but use a more descriptive one by default...but I lean towards more descriptive if it can be accommodated by astropy.io.fits so that future readers of the fits file have a better chance of understanding what was done to it.

Ah, I forgot about HIERARCH card. With that, I'm definitely happy to let fits.io do all the heavy lifting and then we can set it up otherwise how you like.

+1 to it can either be a string or a keyword instance. I'd remove the bit about the value--if people want a specific value than they can pass a Keyword instead. Otherwise this then looks good.

I have:

• Rebased on the latest version in the ccdproc branch to make sure this will be easy to merge.
• Updated to the Keyword class to allow for string values

For the moment I've left the other two options in, but we can pull them if you prefer. The third option definitely makes the most sense to me.