I04_1-165: store and record handle holder barcode separately from pin barcodes. Added unit tests for record and store

Author: IsabellaRey

dls_barcode/data_store/record.py

@@ -29,9 +29,10 @@ class Record:
 
     BAD_SYMBOLS = [EMPTY_SLOT_SYMBOL, NOT_FOUND_SLOT_SYMBOL]
 
-    def __init__(self, plate_type, barcodes, image_path, geometry, timestamp=0.0, id=0):
+    def __init__(self, plate_type, holder_barcode, barcodes, image_path, geometry, timestamp=0.0, id=0):
         """
         :param plate_type: the type of the sample holder plate (string)
+        :param holder_barcode: the barcode of the holder plate
         :param barcodes: ordered array of strings giving the barcodes in each slot
             of the plate in order. Empty slots should be denoted by empty strings.
         :param image_path: the absolute path of the image.
@@ -43,8 +44,10 @@ def __init__(self, plate_type, barcodes, image_path, geometry, timestamp=0.0, id
             self.timestamp = float(timestamp)
         except ValueError:
             self.timestamp = 0.0
+
         self.image_path = image_path
         self.plate_type = plate_type
+        self.holder_barcode = holder_barcode
         self.barcodes = barcodes
         self.geometry = geometry
         self.id = str(id)
@@ -57,27 +60,25 @@ def __init__(self, plate_type, barcodes, image_path, geometry, timestamp=0.0, id
         # Generate timestamp and uid if none are supplied
         if timestamp == 0:
             self.timestamp = time.time()
+
         if id == 0:
             self.id = str(uuid.uuid4())
 
-        # Separate Data and Time
+        # Separate Date and Time
         dt = self._formatted_date().split(" ")
         self.date = dt[0]
         self.time = dt[1]
 
         # Counts of numbers slots and barcodes
-        self.num_slots = len(barcodes)
-        self.num_empty_slots = len([b for b in barcodes if b == EMPTY_SLOT_SYMBOL])
-        self.num_unread_slots = len([b for b in barcodes if b == NOT_FOUND_SLOT_SYMBOL])
+        self.num_slots = len(self.barcodes)
+        self.num_empty_slots = len([b for b in self.barcodes if b == EMPTY_SLOT_SYMBOL])
+        self.num_unread_slots = len([b for b in self.barcodes if b == NOT_FOUND_SLOT_SYMBOL])
         self.num_valid_barcodes = self.num_slots - self.num_unread_slots - self.num_empty_slots
 
     @staticmethod
-    def from_plate(plate, second_plate, image_path):
-        plate_type = second_plate.type
-        geometry = second_plate.geometry()
-        barcodes = plate.barcodes() + second_plate.barcodes()
-
-        return Record(plate_type=plate_type, barcodes=barcodes, image_path=image_path, geometry=geometry)
+    def from_plate(holder_barcode, plate, image_path):
+        return Record(plate_type=plate.type, holder_barcode=holder_barcode, barcodes=plate.barcodes(),
+                      image_path=image_path, geometry=plate.geometry())
 
     @staticmethod
     def from_string(string):
@@ -89,21 +90,24 @@ def from_string(string):
         timestamp = items[Record.IND_TIMESTAMP] #used to convert into float twice
         image = items[Record.IND_IMAGE]
         plate_type = items[Record.IND_PLATE]
-        barcodes = items[Record.IND_BARCODES].split(Record.BC_SEPARATOR)
+        all_barcodes = items[Record.IND_BARCODES].split(Record.BC_SEPARATOR)
+        holder_barcode = all_barcodes[0]
+        pin_barcodes = all_barcodes[1:]
 
         geo_class = Geometry.get_class(plate_type)
         geometry = geo_class.deserialize(items[Record.IND_GEOMETRY])
 
-        return Record(plate_type=plate_type, barcodes=barcodes, timestamp=timestamp,
+        return Record(plate_type=plate_type, holder_barcode=holder_barcode, barcodes=pin_barcodes, timestamp=timestamp,
                       image_path=image, id=id, geometry=geometry)
 
     def to_csv_string(self):
         """ Converts a scan record object into a string that can be stored in a csv file.
         """
-        items = [0] * 3
-        items[0] = str(self.id)
-        items[1] = str(self._formatted_date())
-        items[2] = Record.BC_SEPARATOR.join(self.barcodes)
+        items = list()
+        items.append(str(self.id))
+        items.append(str(self._formatted_date()))
+        items.append(self.holder_barcode)
+        items.append(Record.BC_SEPARATOR.join(self.barcodes))
         return Record.BC_SEPARATOR.join(items)
 
     def to_string(self):
@@ -115,29 +119,28 @@ def to_string(self):
         items[Record.IND_TIMESTAMP] = str(self.timestamp)
         items[Record.IND_IMAGE] = self.image_path
         items[Record.IND_PLATE] = self.plate_type
-        items[Record.IND_BARCODES] = Record.BC_SEPARATOR.join(self.barcodes)
+        items[Record.IND_BARCODES] = Record.BC_SEPARATOR.join(self._all_barcodes())
         items[Record.IND_GEOMETRY] = self.geometry.serialize()
         return Record.ITEM_SEPARATOR.join(items)
 
     def any_barcode_matches(self, barcodes):
         """ Returns true if the record contains any barcode which is also
         contained in the specified list
         """
-        barcodes = [bc for bc in barcodes if bc not in Record.BAD_SYMBOLS]
-        for bc in barcodes:
-            if bc in self.barcodes:
+        valid_barcodes = [bc for bc in barcodes if bc not in Record.BAD_SYMBOLS]
+        for bc in valid_barcodes:
+            if bc in self._all_barcodes():
                 return True
 
         return False
 
-    #TODO: modify when two images merged
-    # only the image of the top for the time being
+    def _all_barcodes(self):
+        return [self.holder_barcode] + self.barcodes
+
     def image(self):
         image = Image.from_file(self.image_path)
         return image
 
-    #TODO: modify when two images merged
-    # only the image of the top for the time being
     def marked_image(self, options):
         geo = self.geometry
         image = self.image()
@@ -155,9 +158,7 @@ def marked_image(self, options):
 
     # marking the top image
     def _draw_pins(self, image, geometry, options):
-        top_barcodes = list(self.barcodes) #copy of the list
-        top_barcodes.pop(0)# remove the first element (side barcode)
-        for i, bc in enumerate(top_barcodes):
+        for i, bc in enumerate(self.barcodes):
             if bc == NOT_FOUND_SLOT_SYMBOL:
                 color = options.col_bad()
             elif bc == EMPTY_SLOT_SYMBOL:

dls_barcode/data_store/store.py

@@ -8,10 +8,10 @@ class Store:
     """ Maintains a list of records of previous barcodes scans. Any changes (additions
     or deletions) are automatically written to the backing file.
     """
-    def __init__(self, directory, options, file_manager):
+    def __init__(self, directory, store_capacity, file_manager):
         """ Initializes a new instance of Store.
         """
-        self._options = options
+        self._store_capacity = store_capacity
         self._directory = directory
         self._file_manager = file_manager
         self._file = os.path.join(directory, "store.txt")
@@ -52,42 +52,41 @@ def get_record(self, index):
         """
         return self.records[index] if self.records else None
 
-    def add_record(self, plate, second_plate, holder_img, pins_img):
+    def _add_record(self, holder_barcode, plate, holder_img, pins_img):
         """ Add a new record to the store and save to the backing file.
         """
         merged_img = self._merge_holder_image_into_pins_image(holder_img, pins_img)
         guid = str(uuid.uuid4())
         filename = os.path.abspath(os.path.join(self._img_dir, guid + '.png'))
         merged_img.save_as(filename)
 
-        record = Record.from_plate(plate, second_plate, filename)
+        record = Record.from_plate(holder_barcode, plate, filename)
 
         self.records.append(record)
         self._process_change()
         self._truncate_record_list()
 
-    def merge_record(self, plate, second_plate, holder_img, pins_img):
-        """ Create new record or replace existing record if it has the same barcodes as the most
+    def merge_record(self, holder_barcode, plate, holder_img, pins_img):
+        """ Create new record or replace existing record if it has the same holder barcode as the most
         recent record. Save to backing store. """
-        #Checking only holder barcodes is sufficient
-        if len(self.records) > 0 and self.records[0].any_barcode_matches(plate.barcodes()):
+        if self.records and self.records[0].holder_barcode == holder_barcode:
             self.delete_records([self.records[0]])
 
-        self.add_record(plate, second_plate, holder_img, pins_img)
+        self._add_record(holder_barcode, plate, holder_img, pins_img)
 
-    def delete_records(self, records):
+    def delete_records(self, records_to_delete):
         """ Remove all of the records in the supplied list from the store and
         save changes to the backing file.
         """
-        for record in records:
+        for record in records_to_delete:
             self.records.remove(record)
             if self._file_manager.is_file(record.image_path):
                 self._file_manager.remove(record.image_path)
 
         self._process_change()
 
     def _truncate_record_list(self):
-        number = self._options.store_capacity.value()
+        number = self._store_capacity.value()
         number = max(number, 2)
 
         if len(self.records) > number:
@@ -102,7 +101,7 @@ def _process_change(self):
         self._to_csv_file()
 
     def _sort_records(self):
-        """ Sort the records in descending date order (must recent first).
+        """ Sort the records in descending date order (most recent first).
         """
         self.records.sort(reverse=True, key=lambda record: record.timestamp)
 

dls_barcode/gui/record_table.py

@@ -67,9 +67,9 @@ def _init_ui(self, to_run_on_table_clicked):
 
         self.setLayout(vbox)
 
-    def add_record_frame(self, plate, second_plate, holder_img, pins_img):
+    def add_record_frame(self, holder_barcode, plate, holder_img, pins_img):
         """ Add a new scan frame - creates a new record if its a new puck, else merges with previous record"""
-        self._store.merge_record(plate, second_plate, holder_img, pins_img)
+        self._store.merge_record(holder_barcode, plate, holder_img, pins_img)
         self._load_store_records()
         if self._options.scan_clipboard.value():
             self._barcodeTable.copy_selected_to_clipboard()

tests/unit_tests/test_dls_barcode/test_data_store/test_record.py

@@ -1,13 +1,20 @@
 import unittest
+from mock import MagicMock
 from dls_barcode.data_store.record import Record
 from dls_barcode.geometry.blank import BlankGeometry
+from dls_barcode.plate import NOT_FOUND_SLOT_SYMBOL, EMPTY_SLOT_SYMBOL
 
 class TestRecord(unittest.TestCase):
 
     def test_from_string_creates_new_record_with_all_given_parameters(self):
-        str = "f59c92c1;1494238920.0;test.png;None;DLSL-010,DLSL-011,DLSL-012;1569:1106:70-2307:1073:68-1944:1071:68"
+        # Arrange
+        str = "f59c92c1;1494238920.0;test.png;None;DLSL-009,DLSL-010,DLSL-011,DLSL-012;1569:1106:70-2307:1073:68-1944:1071:68"
+
+        # ACt
         r = Record.from_string(str)
 
+        # Assert
+        self.assertEqual(r.holder_barcode, 'DLSL-009')
         barcodes = r.barcodes
         self.assertEqual(len(barcodes), 3)
         self.assertTrue('DLSL-010' in barcodes)
@@ -17,11 +24,15 @@ def test_from_string_creates_new_record_with_all_given_parameters(self):
         self.assertEquals(r.plate_type, 'None')
         self.assertTrue(isinstance(r.geometry, BlankGeometry))
 
-
     def test_from_string_creates_new_record_time_stamp_missing(self):
-        str = "f59c92c1; ;test.png;None;DLSL-010,DLSL-011,DLSL-012;1569:1106:70-2307:1073:68-1944:1071:68"
+        # Arrange
+        str = "f59c92c1; ;test.png;None;DLSL-009,DLSL-010,DLSL-011,DLSL-012;1569:1106:70-2307:1073:68-1944:1071:68"
+
+        # Act
         r = Record.from_string(str)
 
+        # Assert
+        self.assertEqual(r.holder_barcode, 'DLSL-009')
         barcodes = r.barcodes
         self.assertEqual(len(barcodes), 3)
         self.assertTrue('DLSL-010' in barcodes)
@@ -32,37 +43,47 @@ def test_from_string_creates_new_record_time_stamp_missing(self):
         self.assertTrue(isinstance(r.geometry, BlankGeometry))
 
     def test_to_string_recreates_given_values_excluding_serial_number(self):
+        # Arrange
         str = "f59c92c1;1494238920.0;test.png;None;DLSL-010,DLSL-011,DLSL-012;1569:1106:70-2307:1073:68-1944:1071:68"
         r = Record.from_string(str)
+
+        # Act
         new_str = r.to_string()
+
+        # Assert
         list_str = str.split(Record.ITEM_SEPARATOR)
         list_new_str = new_str.split(Record.ITEM_SEPARATOR)
-        i = 0
-        while (i < len(list_str)-1):
+        for i in range(len(list_str) - 1):
             self.assertTrue(list_str[i] == list_new_str[i])
-            i = i+1
 
     def test_any_barcode_matches_returns_true_if_only_one_barcode_matches(self):
+        # Arrange
         str = "f59c92c1;1494238920.0;test.png;None;DLSL-010,DLSL-011,DLSL-012;1569:1106:70-2307:1073:68-1944:1071:68"
         r = Record.from_string(str)
 
+        # Act-Assert
         self.assertTrue(r.any_barcode_matches(['DLSL-010','DLSL-011111']))
 
-    def test_any_barcode_matches_returns_false_if_no_barcode_match(self):
+    def test_any_barcode_matches_returns_false_if_no_barcode_matches(self):
+        # Arrange
         str = "f59c92c1;1494238920.0;test.png;None;DLSL-010,DLSL-011,DLSL-012;1569:1106:70-2307:1073:68-1944:1071:68"
         r = Record.from_string(str)
 
+        # Act-Assert
         self.assertFalse(r.any_barcode_matches(['DLSL', 'DLSL3']))
 
     def test_any_barcode_matches_returns_false_if_no_barcodes_passed(self):
+        # Arrange
         str = "f59c92c1;1494238920.0;test.png;None;DLSL-010,DLSL-011,DLSL-012;1569:1106:70-2307:1073:68-1944:1071:68"
         r = Record.from_string(str)
 
+        # Act=Assert
         self.assertFalse(r.any_barcode_matches([]))
 
     def test_csv_string_contains_time_in_human_readable_format(self):
         # Arrange
         timestamp = 1505913516.3836024
+
         # Full human readable timestamp is "2017-09-20 14:18:36" but Travis CI server runs on a different time zone so can't compare the hours
         human_readable_timestamp_day = "2017-09-20 "
         human_readable_timestamp_minutes = ":18:36"
@@ -77,6 +98,48 @@ def test_csv_string_contains_time_in_human_readable_format(self):
         self.assertIn(human_readable_timestamp_minutes, csv_string)
         self.assertNotIn(str(timestamp), csv_string)
 
+    def test_record_can_be_constructed_from_plate_info(self):
+        # Arrange
+        plate_type = "plate type"
+        holder_barcode = "ABCD"
+        barcodes = ["barcode1", "barcode2"]
+        image_path = "a_path"
+        mock_geometry = MagicMock()
+        mock_plate = self._create_mock_plate(plate_type, barcodes, mock_geometry)
+
+        # Act
+        r = Record.from_plate(holder_barcode, mock_plate, image_path)
+
+        # Assert
+        self.assertIsNotNone(r.timestamp)
+        self.assertEquals(r.image_path, image_path)
+        self.assertEquals(r.plate_type, plate_type)
+        self.assertEquals(r.holder_barcode, holder_barcode)
+        self.assertListEqual(r.barcodes, barcodes)
+        self.assertEquals(r.geometry, mock_geometry)
+        self.assertIsNotNone(r.id)
+
+    def test_the_number_of_slots_are_counted_correctly_from_the_pin_barcodes(self):
+        # Arrange
+        barcodes = ["barcode1", "barcode2", EMPTY_SLOT_SYMBOL, "barcode3", NOT_FOUND_SLOT_SYMBOL, EMPTY_SLOT_SYMBOL]
+        mock_plate = self._create_mock_plate("plate type", barcodes, MagicMock())
+
+        # Act
+        r = Record.from_plate(holder_barcode="ABCD", plate=mock_plate, image_path="a_path")
+
+        # Assert
+        self.assertEquals(r.num_slots, len(barcodes))
+        self.assertEquals(r.num_empty_slots, 2)
+        self.assertEquals(r.num_unread_slots, 1)
+        self.assertEquals(r.num_valid_barcodes, 3)
+
+    def _create_mock_plate(self, plate_type, barcodes, geometry):
+        mock_plate = MagicMock()
+        mock_plate.type = plate_type
+        mock_plate.barcodes.return_value = barcodes
+        mock_plate.geometry.return_value = geometry
+        return mock_plate
+