Release 1.7.0 (#77)

* Add support for instrument types (#72) * StepRunner support for running the same step multiple times within a protocol (#73) * Support the Leave in QC Protocol next step action (#74) * Allow custom file name in File.replace_and_commit_from_local() (#75) * Support adding an eSignature to a step from a StepRunner (#76) * Update protocol.py (#65) * Adding logic to programmatically clear pools on a step (#63) * Update changelog for v1.7.0 --------- Co-authored-by: Brian Griffiths <[email protected]>
SemaphoreSolutions · Mar 7, 2025 · 7b04248 · 7b04248
1 parent aa4db0e
commit 7b04248
Show file tree

Hide file tree

Showing 14 changed files with 296 additions and 81 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,22 @@
 Release History
 ===============
+1.7.0
+-
+- (#71) Added support for instrument types:
+   - Added a new `InstrumentType` class
+   - Modified the `Instrument` class to return `InstrumentType` objects when reading the `instrument_type` property
+   - Modified the `StepDetails` class to return `InstrumentType` objects when reading the `permitted_instrument_types` property
+   - Added a new `instrument_types` property to the `LIMS` class that can query instrument types.
+   - The `Instrument` class now correctly reports instrument limsids.
+   - The `instrument_used` property of `StepDetails` is now writable.
+- Several updates to `StepRunner`:
+   - (#69) Step runners now support running the same step multiple times within a protocol.
+   - (#76) Step runners can now sign steps that require an eSignature, by calling `self.sign()` from the `record_details()` method.
+- (#68) The "Leave in QC Protocol" artifact action can now be selected at the Next Steps screen by using `step.actions.artifact_actions[artifact].leave_in_qc_protocol()`
+- (#67) The `replace_and_commit_from_local` method of the `File` class now supports an optional `name` parameter, allowing you to upload a file to Clarity using a different name from the on-disk filename.
+- (#65) Fixes a bug in `StepConfiguration.permitted_containers()` that caused steps with no permitted containers (i.e. no-output steps) to return all container types instead of none. 
+- (#63) Added the `Step.clear_pools()` method which can be used to remove any existing pools that were created on a step.
+
 1.6.1
 -
 - Explicitly declare requests >= 2.22.0 and urllib3 >= 1.25.2 as dependencies, which fixes an edge case causing the `lims.versions` and `lims.current_minor_version` properties to raise an exception on early versions of Python 3.6.

diff --git a/docs/api/clarity.rst b/docs/api/clarity.rst
@@ -118,6 +118,13 @@ Instrument
     :members:
     :show-inheritance:
 
+Instrument Type
+---------------
+
+.. autoclass:: s4.clarity.configuration.InstrumentType
+    :members:
+    :show-inheritance:
+
 IO Map
 ------
 

diff --git a/s4/clarity/configuration/__init__.py b/s4/clarity/configuration/__init__.py
@@ -6,9 +6,11 @@
 from .process_type import ProcessType, ProcessTemplate, Automation
 from .udf import Udf
 from .stage import Stage
+from .instrument_type import InstrumentType
 
 module_members = [
     Automation,
+    InstrumentType,
     ProcessTemplate,
     ProcessType,
     Protocol,

diff --git a/s4/clarity/configuration/instrument_type.py b/s4/clarity/configuration/instrument_type.py
@@ -0,0 +1,18 @@
+# Copyright 2025 Semaphore Solutions, Inc.
+# ---------------------------------------------------------------------------
+
+from s4.clarity._internal import ClarityElement
+from s4.clarity._internal.props import subnode_links, subnode_property
+from .process_type import ProcessType
+
+
+class InstrumentType(ClarityElement):
+    """
+    Reference: https://d10e8rzir0haj8.cloudfront.net/latest/data_itp.html#element_instrument-type
+    """
+
+    UNIVERSAL_TAG = "{http://genologics.com/ri/instrumenttype}instrument-type"
+
+    name = subnode_property("name")
+    vendor = subnode_property("vendor")
+    process_types = subnode_links(ProcessType, "process-type", "process-types")
diff --git a/s4/clarity/configuration/protocol.py b/s4/clarity/configuration/protocol.py
@@ -3,10 +3,17 @@
 
 import logging
 
+from s4.clarity._internal.factory import MultipleMatchingElements
 from s4.clarity._internal.element import ClarityElement, WrappedXml
 from s4.clarity.reagent_kit import ReagentKit
 from s4.clarity.control_type import ControlType
-from s4.clarity._internal.props import subnode_property_list_of_dicts, subnode_property, subnode_property_literal_dict, attribute_property, subnode_element_list
+from s4.clarity._internal.props import (
+    subnode_property_list_of_dicts,
+    subnode_property,
+    subnode_property_literal_dict,
+    attribute_property,
+    subnode_element_list,
+)
 from s4.clarity import types, lazy_property
 
 log = logging.getLogger(__name__)
@@ -15,19 +22,19 @@
 class Protocol(ClarityElement):
     UNIVERSAL_TAG = "{http://genologics.com/ri/protocolconfiguration}protocol"
 
-    properties = subnode_property_literal_dict('protocol-properties', 'protocol-property')
+    properties = subnode_property_literal_dict("protocol-properties", "protocol-property")
     index = attribute_property("index", typename=types.NUMERIC)
 
     @lazy_property
     def steps(self):
         """
         :type: list[StepConfiguration]
         """
-        return [StepConfiguration(self, n) for n in self.xml_findall('./steps/step')]
+        return [StepConfiguration(self, n) for n in self.xml_findall("./steps/step")]
 
     def _step_node(self, name):
-        for n in self.xml_findall('./steps/step'):
-            if n.get('name') == name:
+        for n in self.xml_findall("./steps/step"):
+            if n.get("name") == name:
                 return n
         return None
 
@@ -36,18 +43,24 @@ def step_from_id(self, stepid):
         :rtype: StepConfiguration or None
         """
         for step in self.steps:
-            if step.uri.split('/')[-1] == stepid:
+            if step.uri.split("/")[-1] == stepid:
                 return step
         return None
 
     def step(self, name):
         """
         :rtype: StepConfiguration or None
         """
-        for step in self.steps:
-            if step.name == name:
-                return step
-        return None
+        candidate_steps = [step for step in self.steps if step.name == name]
+        if len(candidate_steps) == 1:
+            return candidate_steps[0]
+        elif len(candidate_steps) < 1:
+            return None
+        else:  # found more than 1 step with the same name
+            raise MultipleMatchingElements(
+                "Multiple steps were found with the name '%s' in the protocol "
+                "'%s'" % (name, self.name)
+            )
 
     @property
     def number_of_steps(self):
@@ -73,18 +86,20 @@ def __init__(self, protocol, node):
         super(StepConfiguration, self).__init__(protocol.lims, uri=None, xml_root=node)
         self.protocol = protocol
 
-    properties = subnode_property_literal_dict('step-properties', 'step-property')
+    properties = subnode_property_literal_dict("step-properties", "step-property")
     protocol_step_index = subnode_property("protocol-step-index", types.NUMERIC)
     queue_fields = subnode_element_list(ProtocolStepField, "queue-fields", "queue-field")
     step_fields = subnode_element_list(ProtocolStepField, "step-fields", "step-field")
     sample_fields = subnode_element_list(ProtocolStepField, "sample-fields", "sample-field")
-    triggers = subnode_property_list_of_dicts('epp-triggers/epp-trigger', as_attributes=[
-        'status', 'point', 'type', 'name'
-    ])
-    transitions = subnode_property_list_of_dicts('transitions/transition', as_attributes=[
-        'name', 'sequence', 'next-step-uri'
-    ], order_by=lambda x: int(x.get('sequence')))
-
+    triggers = subnode_property_list_of_dicts(
+        "epp-triggers/epp-trigger", as_attributes=["status", "point", "type", "name"]
+    )
+    transitions = subnode_property_list_of_dicts(
+        "transitions/transition",
+        as_attributes=["name", "sequence", "next-step-uri"],
+        order_by=lambda x: int(x.get("sequence")),
+    )
+
     def refresh(self):
         """
         :raise Exception: Unable to refresh step directly, use protocol
@@ -103,7 +118,7 @@ def process_type(self):
         """
         :type: ProcessType
         """
-        pt_display_name = self.get_node_text('process-type')
+        pt_display_name = self.get_node_text("process-type")
 
         results = self.lims.process_types.query(displayname=pt_display_name)
         if results:
@@ -134,6 +149,15 @@ def permitted_containers(self):
         """
         container_types = self.xml_findall("./permitted-containers/container-type")
 
+        # a No Outputs step has no permitted containers
+        # but searching for a blank name returns all container types
+        # so we need to check if there are any containers at all
+        # if not, we return an empty list rather than everything
+
+        if len(list(container_types)) == 0:
+            log.warning("No containers found for the step")
+            return []
+
         # container-type (type generic-type-link) has no uri attribute. find the container by name
         # beware if your lims has multiple containers with the same name
 
@@ -146,15 +170,28 @@ def permitted_containers(self):
                 "share the same name?"
             )
 
-        return ret    
-    
+        return ret
+
     @lazy_property
     def permitted_instrument_types(self):
         """
-        :type: List[str]
+        :type: InstrumentType
         """
-        instrument_type_nodes = self.xml_findall("./permitted-instrument-types/instrument-type")
-        return [node.text for node in instrument_type_nodes]
+        instrument_types = self.xml_findall("./permitted-instrument-types/instrument-type")
+
+        # instrument-type (type generic-type-link) has no uri attribute. find the instrument by name
+        # beware if your lims has multiple instruments with the same name
+
+        ret = self.lims.instrument_types.query(name=[i.text for i in instrument_types])
+
+        if len(instrument_types) != len(ret):  # can len(types) > len(ret)?
+            log.warning(
+                "The number of instrument types found differs from the number "
+                "specified in the step config. Do multiple instrument types "
+                "share the same name?"
+            )
+
+        return ret
 
     @lazy_property
     def queue(self):

diff --git a/s4/clarity/file.py b/s4/clarity/file.py
@@ -103,10 +103,12 @@ def pipe_to(self, target_file_object):
 
         target_file_object.write(file_contents)
 
-    def replace_and_commit_from_local(self, local_file_path, content_type='text/plain', mode="r+b"):
+    def replace_and_commit_from_local(self, local_file_path, content_type='text/plain', mode="r+b", name=None):
+        if not name:
+            name = local_file_path
         self.mode = mode
         other_file = open(local_file_path, self.mode)
-        self.replace_and_commit(other_file, local_file_path, content_type)
+        self.replace_and_commit(other_file, name, content_type)
         other_file.close()
 
     def replace_and_commit(self, stream, name, content_type='text/plain'):

diff --git a/s4/clarity/instrument.py b/s4/clarity/instrument.py
@@ -3,39 +3,60 @@
 
 from ._internal import ClarityElement
 from ._internal.props import subnode_property
-from s4.clarity import types, lazy_property
+from s4.clarity import ETree, types, lazy_property
 
 
 class Instrument(ClarityElement):
     """
-    Reference: https://d10e8rzir0haj8.cloudfront.net/5.3/rest.version.instruments.html
-    Note: See example xml_root below --
-        <inst:instrument xmlns:inst="http://genologics.com/ri/instrument" limsid="55-6" uri="https://clarityhost/api/v2/instruments/6">
-        We use uri id for Instrument object limsid i.e. "6" instead of "55-6"
-
-    Cautionary Notes:
-    In most cases, a Step UDF or Reagent Kit/Reagent Lot is a better option to track instrument use in the LIMS.
-    Consider --
-    1. Instrument API endpoint only permits GET
-    2. You may configure multiple Instruments to step. However, you can only assign 1 Instrument per step
-    3. If Instrument is configured to step, Step UDFs CAN NOT be set via the API,
+    Reference: https://d10e8rzir0haj8.cloudfront.net/latest/data_inst.html#instrument
+
+    NOTE: In most cases, a Step UDF or Reagent Kit/Reagent Lot is a better
+    option to track instrument use in the LIMS. If you choose to use
+    Instruments, be aware that there are a number of caveats:
+
+    1. You may configure multiple Instruments to step. However, you can only
+    assign 1 Instrument per step.
+
+    2. If Instrument is configured to step, Step UDFs CAN NOT be set via the API,
         until the Instrument field is set via UX/UI
-    4. Instruments are not part of the ETL that supports the Illumina Reporting Module
+
+    3. Instruments are not supported by the Illumina Reporting Module
     """
 
     UNIVERSAL_TAG = "{http://genologics.com/ri/instrument}instrument"
 
     name = subnode_property("name")
-    instrument_type = subnode_property("type")
     serial_number = subnode_property("serial-number")
     expiry_date = subnode_property("expiry-date", types.DATE)
     archived = subnode_property("archived", types.BOOLEAN)
 
+    @lazy_property
+    def limsid(self):
+        """:type: str"""
+        if self._limsid is None:
+            if self.xml_root is not None:
+                self._limsid = self._xml_root.get('limsid')
+            else:
+                raise Exception("No limsid available because there is no xml_root set")
+        return self._limsid
+
+    @property
+    def instrument_type(self):
+        """:type: InstrumentType"""
+        return self.lims.instrument_types.get_by_name(self.xml_find("./type").text)
+
+    @instrument_type.setter
+    def instrument_type(self, value):
+        itype = self.xml_find("./type")
+        if itype is None:
+            itype = ETree.SubElement(self.xml_root, "type")
+        itype.text = value.name
+
     @lazy_property
     def related_instruments(self):
         """
         :return: List of instruments of the same instrument type
-        :type: List[Instrument]
+        :type: List[InstrumentType]
         """
         instruments = self.lims.instruments.all()
         return [instrument for instrument in instruments

diff --git a/s4/clarity/lims.py b/s4/clarity/lims.py
@@ -143,7 +143,7 @@ def __init__(self, root_uri, username, password, dry_run=False, insecure=False,
         self.permissions = ElementFactory(self, Permission, batch_flags=BatchFlags.QUERY)
 
         # configuration
-        from .configuration import Workflow, Protocol, ProcessType, Udf, ProcessTemplate, Automation
+        from .configuration import Workflow, Protocol, ProcessType, Udf, ProcessTemplate, Automation, InstrumentType
 
         self.workflows = ElementFactory(self, Workflow, batch_flags=BatchFlags.QUERY,
                                         request_path='/configuration/workflows')
@@ -158,6 +158,9 @@ def __init__(self, root_uri, username, password, dry_run=False, insecure=False,
         self.automations = ElementFactory(self, Automation, batch_flags=BatchFlags.QUERY,
                                           name_attribute="name", request_path="/configuration/automations")
 
+        self.instrument_types = ElementFactory(self, InstrumentType, batch_flags=BatchFlags.QUERY,
+                                          name_attribute="name", request_path="/configuration/instrumenttypes")
+
         self.stages = ElementFactory(self, Stage)
 
     def factory_for(self, element_type):

diff --git a/s4/clarity/step.py b/s4/clarity/step.py
@@ -29,6 +29,7 @@
 REVIEW_ACTION = "review"
 STORE_ACTION = "store"
 NEXT_STEP_ACTION = "nextstep"
+LEAVE_IN_QC_PROTOCOL_ACTION = "leave"
 
 PROGRAM_STATUS_ERROR = "ERROR"
 PROGRAM_STATUS_RUNNING = "RUNNING"
@@ -252,6 +253,12 @@ def __init__(self, lims, step, xml_root):
         super(ArtifactAction, self).__init__(lims, xml_root)
         self.step = step
 
+    def leave_in_qc_protocol(self):
+        """
+        Sets the Next Step property for the artifact specified by artifact_uri to 'Leave in QC Protocol'
+        """
+        self._set_artifact_next_action(LEAVE_IN_QC_PROTOCOL_ACTION)
+
     def remove_from_workflow(self):
         """
         Sets the Next Step property for the artifact specified by artifact_uri to 'Remove From Workflow'
@@ -384,7 +391,7 @@ class StepDetails(IOMapsMixin, FieldsMixin, ClarityElement):
     IOMAPS_OUTPUT_TYPE_ATTRIBUTE = "type"
 
     name = subnode_property("configuration", readonly=True)
-    instrument_used = subnode_link(Instrument, "instrument", readonly=True, attributes=('uri',))
+    instrument_used = subnode_link(Instrument, "instrument", attributes=('uri',))
 
     def __init__(self, step, *args, **kwargs):
         self.step = step
@@ -453,6 +460,14 @@ def create_pool(self, name, samples):
         for sample in samples:
             ETree.SubElement(pool_node, "input", {"uri": sample.uri})
 
+    def clear_pools(self):
+        # type: () -> None
+        """
+        Removes all existing pools that were created on this step.
+        """
+        self.xml_root.remove(self.xml_root.find("./pooled-inputs"))
+        ETree.SubElement(self.xml_root, "pooled-inputs")
+
 
 class AvailableInput(WrappedXml):
     def __init__(self, lims, xml_root):