diff --git a/CHANGELOG.md b/CHANGELOG.md index 07db0ef..2710684 100644 --- 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 index 018452a..8d7fa1f 100644 --- 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 index 694b4c8..a03fdb1 100644 --- 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 new file mode 100644 index 0000000..2111703 --- /dev/null +++ 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 index b356f09..4887456 100644 --- 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,7 +22,7 @@ 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 @@ -23,11 +30,11 @@ 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,7 +43,7 @@ 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 @@ -44,10 +51,16 @@ 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 index 50d8304..42482e0 100644 --- 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 index 3fa3bd6..d6664be 100644 --- 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 -- - - 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 index 3113807..67c666b 100644 --- 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 index f8fb746..13069b3 100644 --- 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): diff --git a/s4/clarity/steputils/step_runner.py b/s4/clarity/steputils/step_runner.py index be43465..be80a03 100644 --- a/s4/clarity/steputils/step_runner.py +++ b/s4/clarity/steputils/step_runner.py @@ -5,10 +5,17 @@ import functools import time import logging +try: + from urllib.parse import urlparse, urlunparse # Python 3 +except ImportError: + from urlparse import urlparse, urlunparse # Python 2 + +from requests import Session from s4.clarity import ETree, lazy_property from s4.clarity.step import Step from s4.clarity import ClarityException +from s4.clarity._internal.factory import MultipleMatchingElements log = logging.getLogger(__name__) @@ -44,8 +51,9 @@ class StepRunner: __metaclass__ = abc.ABCMeta - def __init__(self, lims, protocolname, stepconfigname, usequeuedinputs=True, numberofinputs=4): + def __init__(self, lims, protocolname, stepconfigname, usequeuedinputs=True, numberofinputs=4, protocolstepid=None, username=None, password=None): self.step = None + self.protocolstepid = protocolstepid self.lims = lims if self.lims is None: @@ -64,6 +72,20 @@ def __init__(self, lims, protocolname, stepconfigname, usequeuedinputs=True, num self.timeformat = "%I:%S %p" # HH:MM am/pm + # If a username and password were supplied to the StepRunner, use them + # for e-signing. Otherwise use the same username and password that we + # are using for all other API requests. This is to handle the case + # where steps must be signed by a different user than the user who + # started the step. + if username: + self.username = username + else: + self.username = self.lims.username + if password: + self.password = password + else: + self.password = self.lims.password + @lazy_property def step_config(self): """ @@ -71,7 +93,13 @@ def step_config(self): """ try: protocol_config = self.lims.protocols.get_by_name(self.protocolname) - step = protocol_config.step(self.stepconfigname) + try: + step = protocol_config.step(self.stepconfigname) + except MultipleMatchingElements: + if self.protocolstepid: + step = protocol_config.step_from_id(str(self.protocolstepid)) + else: + raise except ClarityException as ex: log.error("StepRunner could not load step configuration: %s" % str(ex)) raise Exception("Configuration for step %s in protocol %s could not be located." % (self.stepconfigname, self.protocolname)) @@ -580,6 +608,55 @@ def prefetch_artifacts(self): """ self.lims.artifacts.batch_refresh(self.step.details.inputs + self.step.details.outputs) + def sign(self): + """ + Adds an eSignature to a step. + Step should be in Record Details phase. + """ + url = urlparse(self.lims.root_uri) + + # CORS headers + headers = { + 'X-Requested-With': 'XMLHttpRequest', + 'Origin': f'{url.scheme}://{url.netloc}' + } + + session = self._get_web_authenticated_session() + work_num = self.step.limsid.split('-')[1] + res = session.post( + urlunparse(url._replace(path=f'/clarity/api/work/{work_num}/e-signature')), + data={'username': self.username, 'password': self.password}, + headers=headers + ) + if not res.ok: + data = res.json() + raise RuntimeError( + f"E-signing failed for step {self.step.limsid}: {data['message']}" + ) + + def _get_web_authenticated_session(self): + """ + Returns a new session that has authenticated to Clarity LIMS + using the web GUI, rather than using the API. This is intentionally + kept separate from self.lims._session so that it can use different + credentials if required. + """ + url = urlparse(self.lims.root_uri) + session = Session() + + if self.lims._insecure: + session.verify = False + + resp = session.post( + urlunparse(url._replace(path='/clarity/j_spring_security_check')), + data={"j_username": self.username, "j_password": self.password} + ) + if not resp.ok: + raise RuntimeError( + f"Failed to authenticate to LIMS as user {self.username}" + ) + + return session class StepRunnerException(Exception): pass diff --git a/s4/clarity/test/generic_testcases.py b/s4/clarity/test/generic_testcases.py index d2bd493..d65a1ad 100644 --- a/s4/clarity/test/generic_testcases.py +++ b/s4/clarity/test/generic_testcases.py @@ -6,30 +6,12 @@ from s4.clarity import ETree -class LimsTestCase(TestCase): - - @staticmethod - def element_from_xml(element_class, xml, **extra_kwargs): - try: - return element_class( - lims=FakeLims(), - xml_root=ETree.fromstring(xml), - **extra_kwargs - ) - except TypeError as e: - str(e) - if "__init__() takes at least" in str(e): - raise TypeError("Unable to instantiate %s, provide extra args in extra_kwargs. %s" - % (element_class.__name__, e)) - else: - raise e - - class FakeLims: def __init__(self): self.artifacts = FakeFactory() self.samples = FakeFactory() self.steps = FakeFactory() + self.instrument_types = FakeFactory() def factory_for(self, element_type): return self.artifacts @@ -39,8 +21,11 @@ class FakeFactory: def __init__(self): self.instances = {} - def from_link_node(self, linknode): + def get_by_name(self, name): + o = FakeElement() + return o + def from_link_node(self, linknode): if linknode is None: return None @@ -74,3 +59,25 @@ def batch_fetch(*args, **kwargs): class FakeElement: def __str__(self): return str(self.__dict__) + + +class LimsTestCase(TestCase): + + @staticmethod + def get_fake_lims(): + return FakeLims() + + def element_from_xml(self, element_class, xml, **extra_kwargs): + try: + return element_class( + lims=self.get_fake_lims(), + xml_root=ETree.fromstring(xml), + **extra_kwargs + ) + except TypeError as e: + str(e) + if "__init__() takes at least" in str(e): + raise TypeError("Unable to instantiate %s, provide extra args in extra_kwargs. %s" + % (element_class.__name__, e)) + else: + raise e diff --git a/s4/clarity/test/s4/clarity/_internal/test_props.py b/s4/clarity/test/s4/clarity/_internal/test_props.py index ab4b73d..6d07de0 100644 --- a/s4/clarity/test/s4/clarity/_internal/test_props.py +++ b/s4/clarity/test/s4/clarity/_internal/test_props.py @@ -1,9 +1,9 @@ import string - try: from collections.abc import MutableSet except ImportError: from collections import MutableSet + from s4.clarity import types, ETree from s4.clarity._internal import WrappedXml from s4.clarity._internal.props import attribute_property, subnode_property, subnode_link, subnode_links, subnode_element, subnode_property_literal_dict, subnode_property_dict, subnode_element_list @@ -47,6 +47,7 @@ class RootElementWrapper(WrappedXml): sub_element = subnode_element(SubNode, "sub_node") sub_element_list = subnode_element_list(SubNode, "sub_nodes", "sub_node") + class PropertysTestCase(LimsTestCase): def test_attribute_property(self): diff --git a/s4/clarity/test/s4/clarity/test_instrument.py b/s4/clarity/test/s4/clarity/test_instrument.py index 0ffa1f1..24c0c45 100644 --- a/s4/clarity/test/s4/clarity/test_instrument.py +++ b/s4/clarity/test/s4/clarity/test_instrument.py @@ -1,12 +1,15 @@ # Copyright 2021 Semaphore Solutions, Inc. # --------------------------------------------------------------------------- +from datetime import date # typing +try: + from unittest.mock import patch +except ImportError: + from mock import patch + from s4.clarity.instrument import Instrument from s4.clarity.step import Step, StepDetails - -from s4.clarity.test.generic_testcases import LimsTestCase - -from datetime import date # typing +from s4.clarity.test.generic_testcases import FakeElement, FakeLims, LimsTestCase from s4.clarity.utils.date_util import str_to_date @@ -34,19 +37,24 @@ def test_instrument_object_properties(self): Point of Interest: Instrument API endpoint only permits GET """ - # Parse the xml into an Instrument object - instrument = self.element_from_xml(Instrument, INSTRUMENT_XML) # type: Instrument + fake_lims = FakeLims() + fake_instrument_type = FakeElement() + + with patch.object(self, "get_fake_lims", return_value=fake_lims): + with patch.object(fake_lims.instrument_types, "get_by_name", return_value=fake_instrument_type): + # Parse the xml into an Instrument object + instrument = self.element_from_xml(Instrument, INSTRUMENT_XML) # type: Instrument - self.assertEqual(instrument.name, "instrument_2") - self.assertEqual(instrument.limsid, "4") - self.assertEqual(instrument.instrument_type, "Instrument ABC") - self.assertEqual(instrument.serial_number, "2222") - self.assertEqual(instrument.archived, False) + self.assertEqual(instrument.name, "instrument_2") + self.assertEqual(instrument.limsid, "55-4") + self.assertEqual(instrument.instrument_type, fake_instrument_type) + self.assertEqual(instrument.serial_number, "2222") + self.assertEqual(instrument.archived, False) - clarity_date_time_str = "2021-12-06" - expected_date = str_to_date(clarity_date_time_str) # type: date + clarity_date_time_str = "2021-12-06" + expected_date = str_to_date(clarity_date_time_str) # type: date - self.assertEqual(instrument.expiry_date, expected_date) + self.assertEqual(instrument.expiry_date, expected_date) STEP_XML = \ diff --git a/s4/clarity/version.py b/s4/clarity/version.py index 754f434..8ee1bf7 100644 --- a/s4/clarity/version.py +++ b/s4/clarity/version.py @@ -1,4 +1,4 @@ # Copyright 2019 Semaphore Solutions, Inc. # --------------------------------------------------------------------------- -__version__ = '1.6.1' +__version__ = '1.7.0'