Added reading each ROI group #26
Conversation
There was a problem hiding this comment.
Thanks for the PR! I have added comments requesting changes to the code formatting. It is bad practice in Python to comment on every line in the code. Many operations are self-descriptive and comments for these only add fluff; comments should be directed towards explaining complex or counter-intuitive lines or blocks of code. Also, we do not need to keep old code commented out. The code is under version control, which is much better for tracking changes and what code used to look like before changes! Additionally, note that trailing spaces should be avoided.
Beyond the cosmetic changes to the main code, we will also need more changes before the PR can be completed.
- Unit tests currently fail, and need to be fixed. In order to fix these, the json files indicating expected outputs will need to be adjusted to contain the expected group value.
- A new unit test will need to be added which makes sure the new code runs correctly for more than one group value (since I expect all the current unit tests will have a group value of 0).
| @@ -316,9 +316,13 @@ def extract_basic_roi_data(data): | |||
| overlayFontSize = get_uint16(data, hdr2Offset + HEADER_OFFSET['OVERLAY_FONT_SIZE']) | |||
| imageOpacity = get_byte(data, hdr2Offset + HEADER_OFFSET['IMAGE_OPACITY']) | |||
| imageSize = get_uint32(data, hdr2Offset + HEADER_OFFSET['IMAGE_SIZE']) | |||
| # abb, read group from file | |||
There was a problem hiding this comment.
I don't understand the need for this comment; this is one of many bytes being loaded all in the same way, and there is no need to comment on this particular one. Not sure why it begins with "abb," either. What does this mean?
| @@ -316,9 +316,13 @@ def extract_basic_roi_data(data): | |||
| overlayFontSize = get_uint16(data, hdr2Offset + HEADER_OFFSET['OVERLAY_FONT_SIZE']) | |||
| imageOpacity = get_byte(data, hdr2Offset + HEADER_OFFSET['IMAGE_OPACITY']) | |||
| imageSize = get_uint32(data, hdr2Offset + HEADER_OFFSET['IMAGE_SIZE']) | |||
| # abb, read group from file | |||
| group = get_byte(data, hdr2Offset + HEADER_OFFSET['AVAILABLE_BYTE1']) | |||
There was a problem hiding this comment.
Please replace HEADER_OFFSET['AVAILABLE_BYTE1'] with HEADER_OFFSET['GROUP'] (both here, and of course also where the offset is defined. This is in keeping with our source code reference.
| # abb, append group to tuple | ||
| #roi_props = (hdr2Offset, n_coordinates, roi_type, channel, slice, frame, position, version, subtype, size) | ||
| roi_props = (hdr2Offset, n_coordinates, roi_type, channel, slice, frame, position, version, subtype, size, group) |
There was a problem hiding this comment.
No need for the comment, or to include the deleted line as a comment.
| # abb, append group to tuple | |
| #roi_props = (hdr2Offset, n_coordinates, roi_type, channel, slice, frame, position, version, subtype, size) | |
| roi_props = (hdr2Offset, n_coordinates, roi_type, channel, slice, frame, position, version, subtype, size, group) | |
| roi_props = (hdr2Offset, n_coordinates, roi_type, channel, slice, frame, position, version, subtype, size, group) |
| # abb, append group to returned (assigned) tuple | ||
| #roi, (hdr2Offset, n_coordinates, roi_type, channel, slice, frame, position, version, subtype, size) = extract_basic_roi_data(data) | ||
| roi, (hdr2Offset, n_coordinates, roi_type, channel, slice, frame, position, version, subtype, size, group) = extract_basic_roi_data(data) |
There was a problem hiding this comment.
No need for this comment, or to include deleted line as a comment.
| # abb, append group to returned (assigned) tuple | |
| #roi, (hdr2Offset, n_coordinates, roi_type, channel, slice, frame, position, version, subtype, size) = extract_basic_roi_data(data) | |
| roi, (hdr2Offset, n_coordinates, roi_type, channel, slice, frame, position, version, subtype, size, group) = extract_basic_roi_data(data) | |
| roi, (hdr2Offset, n_coordinates, roi_type, channel, slice, frame, position, version, subtype, size, group) = extract_basic_roi_data(data) |
| # abb, add group to dict | ||
| roi['group'] = group | ||
|
|
There was a problem hiding this comment.
No need for comment describing self-evident operation. Empty lines should not contain spaces.
| # abb, add group to dict | |
| roi['group'] = group | |
| roi['group'] = group | |
This PR adds reading and return the group for each ROI. The ROI group is useful for segregating ROIs into, well, different groups.
I have not thoroughly tested this code but it is working for my ROI .zip files!!!