-
Notifications
You must be signed in to change notification settings - Fork 13
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
DEV: _get_images can take header or list of headers as input #61
Conversation
Codecov Report
@@ Coverage Diff @@
## master #61 +/- ##
==========================================
- Coverage 28.92% 28.72% -0.21%
==========================================
Files 10 10
Lines 280 282 +2
==========================================
Hits 81 81
- Misses 199 201 +2
Continue to review full report at Codecov.
|
@@ -178,7 +178,10 @@ def get_images_to_3D(images, dtype=None): | |||
|
|||
def _get_images(header, tag, roi=None): | |||
t = ttime.time() | |||
images = header.db.get_images(header, tag) | |||
if isinstance(header, (list, tuple)): |
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.
Can you change this to use header.data
instead?
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.
thanks for suggestion on new api. updated
csxtools/utils.py
Outdated
@@ -178,7 +178,10 @@ def get_images_to_3D(images, dtype=None): | |||
|
|||
def _get_images(header, tag, roi=None): | |||
t = ttime.time() | |||
images = header.db.get_images(header, tag) | |||
if isinstance(header, (list, tuple)): | |||
images = header[0].data(tag) |
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.
Are we only interested in the first image here?
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.
@mrakitin is correct -- we don't want to throw away headers in the case of a list input.
We want to be able to use two (or more generally an arbitrary list of) gain 8 dark images to correct the data and this functionality was broken in the last update.
from databroker import DataBroker as db
from csxtools.utils import get_fastccd_images
old_scan_no = 85528
slicerator = get_fastccd_images(db[old_scan_no], (db[old_scan_no-1, old_scan_no+1], None, None))
i.e. 85527 & 85529 are dark scan_ids with gain 8.
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.
Maybe we can do something like this:
images = []
if isinstance(header, (list, tuple)):
for i in range(len(header)):
images.append(header[i].data(tag))
I'm not sure if there needs to be a corresponding update downstream.
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.
Corrected the code above.
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.
(h.data(tag) for h in header) is not working. It fails at get_images_to_3D function. We need a pims object.
@mpmdean What is the expected signature of this function? |
Below is the error from the latest CSX kernal on srv1. It's trying to access the _get_images method on the list of two headers. This works in older kernals. AttributeError Traceback (most recent call last) /opt/conda_envs/analysis-2018-1.0/lib/python3.6/site-packages/csxtools/utils.py in get_fastccd_images(light_header, dark_headers, flat, gain, tag, roi) /opt/conda_envs/analysis-2018-1.0/lib/python3.6/site-packages/csxtools/utils.py in _get_images(header, tag, roi) AttributeError: 'list' object has no attribute 'db' I believe this comes from this change: -def _get_images(header, tag, roi=None, handler_override=None): images = get_images(header, tag, handler_override=handler_override) |
I think this should work. I also did some local test. any more 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.
Looks good to me! Thanks @licode
Hi All, |
I am not sure what happened, but now this functionality seems to work. What is very scary is that the version of csxtools remains the same so I don't really understand why I had the issue in the first place. I tried all 3 23id1 servers before and found this issue. But just this week, I forgot and ran other notebooks and it seemed to be fixed. I am going to close the issue on csxtools |
This is to be compatible with previous api that _get_images can take list of headers as input.