Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

status flag added to fits headers #38

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

status flag added to fits headers #38

wants to merge 2 commits into from

Conversation

wilawerek
Copy link

Now pp_prepare, pp_register, pp_photometry, pp_calibrate and pp_distill add keywords PP_REGIS, PP_PHOTO, PP_CALIB and PP_DISTI with values STARTED (at the beginning of the script) or SUCCESS (at the end of the script) to fits headers.

Copy link
Owner

@mommermi mommermi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Emil, this looks good! Thanks for working on this! I just have a few short comments

pp_calibrate.py Outdated
try:
header.set('PP_CALIB', 'STARTED', 'PP: pp_calibrate status flag',
after='PP_PHOTO')
except:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, please try to avoid except without listing a specific error that you would like to intercept. What kind of error could occur that would need to be intercepted here? I guess the only issue might be that PP_PHOTO does not exist, so that would probably be a KeyError. Anything else?

pp_calibrate.py Outdated
after='PP_PHOTO')
except:
print(('%s image header incomplete, have the data run ' +
'through pp_prepare?') % filename)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't pp_photometry run right before pp_calibrate?

pp_calibrate.py Outdated
header = hdulist[0].header
try:
header.set('PP_CALIB', 'SUCCESS')
except:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that there should be need to intercept errors, since PP_CALIB already exists in the header.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... the same applies to the other functions...

pp_calibrate.py Outdated
# add flag keyword to header and set it to STARTED
header = hdulist[0].header
try:
header.set('PP_CALIB', 'STARTED', 'PP: pp_calibrate status flag',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thinking about it, shouldn't we set the keyword to FAILED by default? If the function breaks, it will stick with FAILED, or else it will switch to SUCCESS. Can you change that in this function and the others?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants