-
Notifications
You must be signed in to change notification settings - Fork 50
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
Only the primary camera should have images saved in list #329
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #329 +/- ##
===========================================
+ Coverage 83.81% 83.84% +0.03%
===========================================
Files 51 51
Lines 3367 3368 +1
Branches 422 422
===========================================
+ Hits 2822 2824 +2
Misses 404 404
+ Partials 141 140 -1
Continue to review full report at Codecov.
|
@@ -143,7 +143,8 @@ def take_observation(self, observation, headers=None, filename=None, **kwargs): | |||
proc = self.take_exposure(seconds=exp_time, filename=file_path) | |||
|
|||
# Add most recent exposure to list | |||
observation.exposure_list[image_id] = file_path.replace('.cr2', '.fits') | |||
if self.is_primary: | |||
observation.exposure_list[image_id] = file_path.replace('.cr2', '.fits') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the point that we're trying to have a single canonical image to represent each exposure, even if we have multiple cameras? I'm not sure I understand why. Perhaps a comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exposure list is used for tracking purposes so the closed-loop compares the pointing image to the last_exposure
. We always want to compare just the primary camera to itself.
I was going to expand exposure_list
so it contained all exposures, which would make the cleanup and uploading to cloud a bit easier. So this might change going forward.
I'll add an explanation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took the easy out and made #342 for now. I want to make the whole thing more robust.
The
expsoure_list
is used to get thelast_exposure
, which is then used for pointing comparisons. We only want to track the primary camera for this.