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

Tickets/dm 5503 #32

Merged
merged 3 commits into from Oct 5, 2016
Merged

Tickets/dm 5503 #32

merged 3 commits into from Oct 5, 2016

Conversation

r-owen
Copy link
Contributor

@r-owen r-owen commented Sep 28, 2016

No description provided.

Copy link
Contributor

@PaulPrice PaulPrice left a comment

Choose a reason for hiding this comment

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

A few things to think about. Please include darktime in VisitInfo.

# Keywords EXPTIME and MJD-OBS are used to set the calib object.
for kw in ('EXPTIME', 'MJD-OBS', 'DARKTIME'):
# extra keywords to copy to the exposure
for kw in ('DARKTIME', ):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not include darktime in VisitInfo? It's required for the dark correction, and it would be good not to have to grab it from the header in ISR.

exp.getCalib().setExptime(1.0)
rawPath = self.map_raw(dataId).getLocations()[0]
headerPath = re.sub(r'[\[](\d+)[\]]$', "[0]", rawPath)
md0 = afwImage.readMetadata(headerPath)
Copy link
Contributor

Choose a reason for hiding this comment

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

@hchiang2 should comment here about whether the darks are MEFs or not. I suspect they are not any more.

Copy link

@hsinfang hsinfang Sep 29, 2016

Choose a reason for hiding this comment

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

This dark is made by pipe_drivers constructDark.py, so no, it's not a MEF.

This is to get the raw header though, and raws are MEFs.

calib.setFluxMag0(10**(0.4 * md0.get("MAGZERO")))
exp.setCalib(calib)

visitInfo = self._makeRawVisitInfo(md0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the underscore on the method name? To me, the underscore says "you should never have to call this method because it's an internal implementation detail", yet you are calling it.

@@ -191,7 +191,8 @@ def bypass_instcal(self, datasetType, pythonType, butlerLocation, dataId):
calib = afwImage.Calib()
calib.setFluxMag0(10**(0.4 * md0.get("MAGZERO")))
exp.setCalib(calib)
visitInfo = self._makeRawVisitInfo(md0)
exposureId = self._computeCcdExposureId(dataId)
visitInfo = self._makeRawVisitInfo(md=md0, exposureId=exposureId)
Copy link
Contributor

Choose a reason for hiding this comment

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

Squash into previous commit?

@@ -233,7 +241,11 @@ def std_raw(self, item, dataId):

def std_dark(self, item, dataId):
exp = afwImage.makeExposure(afwImage.makeMaskedImage(item))
exp.getCalib().setExptime(1.0)
rawPath = self.map_raw(dataId).getLocations()[0]
Copy link

@hsinfang hsinfang Sep 29, 2016

Choose a reason for hiding this comment

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

Would it ever be possible that one tries to get a dark while raw of the same dataId is not available? (problems using map_raw in std_dark? )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose it is possible, but I think it unlikely. It should not happen during ISR.

DecamMapper sets VisitInfo in raw images and instcal images
@r-owen
Copy link
Contributor Author

r-owen commented Sep 30, 2016

All suggestions implemented.

@r-owen r-owen merged commit 2a01b0d into master Oct 5, 2016
@ktlim ktlim deleted the tickets/DM-5503 branch August 25, 2018 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants