From 409373d882eb956671542860914119b4227d0731 Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Tue, 4 Aug 2026 13:01:23 +1000 Subject: [PATCH] Assign / unassign sequence should be specific to sequence type I previously incorrectly believed that sequence types are mutually exclusive. E.g. you can have max 1 relationship between two tasks. Now after looking at more schedules I realise it's logically allowed to have more than one sequence relationship. For example simultaneous SS + FF. --- .../api/sequence/assign_sequence.py | 10 +- .../api/sequence/duplicate_task.py | 1 + .../api/sequence/unassign_sequence.py | 24 +++- .../test/api/sequence/test_assign_sequence.py | 95 +++++++++++++++ .../api/sequence/test_unassign_sequence.py | 111 ++++++++++++++++++ 5 files changed, 235 insertions(+), 6 deletions(-) create mode 100644 src/ifcopenshell-python/test/api/sequence/test_assign_sequence.py create mode 100644 src/ifcopenshell-python/test/api/sequence/test_unassign_sequence.py diff --git a/src/ifcopenshell-python/ifcopenshell/api/sequence/assign_sequence.py b/src/ifcopenshell-python/ifcopenshell/api/sequence/assign_sequence.py index 65491604aa..19599165b2 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/sequence/assign_sequence.py +++ b/src/ifcopenshell-python/ifcopenshell/api/sequence/assign_sequence.py @@ -106,8 +106,16 @@ def assign_sequence( # to be 2000-01-05. ifcopenshell.api.sequence.cascade_schedule(model, task=formwork) """ + # Matched on the type as well as the pair. Two tasks may legitimately be + # sequenced more than once with different types — the classic case is a + # "ladder", where a start to start lets the follower begin once the leader + # has begun and a finish to finish stops it ending before the leader ends. + # Both constraints are real and neither implies the other, so matching on + # the pair alone silently hands back the wrong relationship: a caller adding + # a second sequence would instead be handed the first, and editing the type + # on it would destroy the constraint that was already there. for rel in related_process.IsSuccessorFrom or []: - if rel.RelatingProcess == relating_process: + if rel.RelatingProcess == relating_process and rel.SequenceType == sequence_type: return rel rel = file.create_entity( "IfcRelSequence", diff --git a/src/ifcopenshell-python/ifcopenshell/api/sequence/duplicate_task.py b/src/ifcopenshell-python/ifcopenshell/api/sequence/duplicate_task.py index db9ed0f7db..bc2811da67 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/sequence/duplicate_task.py +++ b/src/ifcopenshell-python/ifcopenshell/api/sequence/duplicate_task.py @@ -156,6 +156,7 @@ class Usecase: self.file, relating_process=relating_process, related_process=related_process, + sequence_type=inverse.SequenceType, ) if inverse.TimeLag: ifcopenshell.api.sequence.assign_lag_time( diff --git a/src/ifcopenshell-python/ifcopenshell/api/sequence/unassign_sequence.py b/src/ifcopenshell-python/ifcopenshell/api/sequence/unassign_sequence.py index 15ed7d3cf1..8d3c6e9186 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/sequence/unassign_sequence.py +++ b/src/ifcopenshell-python/ifcopenshell/api/sequence/unassign_sequence.py @@ -19,17 +19,28 @@ import ifcopenshell import ifcopenshell.api.sequence import ifcopenshell.util.element +from typing import Optional def unassign_sequence( file: ifcopenshell.file, relating_process: ifcopenshell.entity_instance, related_process: ifcopenshell.entity_instance, + sequence_type: Optional[str] = None, ) -> None: """Removes a sequence relationship between tasks + Two tasks may be sequenced more than once with different types — see + :func:`ifcopenshell.api.sequence.assign_sequence` — so removing "the" + relationship between a pair is ambiguous. Left unspecified, every sequence + between the two is removed, which is what "make them unrelated" means and + what this did before the parameter existed. Name a type to remove only that + one and leave any others in place. + :param relating_process: The previous / predecessor task. :param related_process: The next / successor task. + :param sequence_type: Optionally, remove only the sequence of this type. + Choose from FINISH_START, FINISH_FINISH, START_START, or START_FINISH. :return: None Example: @@ -58,9 +69,12 @@ def unassign_sequence( relating_process=zone1, related_process=zone2) """ for rel in related_process.IsSuccessorFrom or []: - if rel.RelatingProcess == relating_process: - history = rel.OwnerHistory - file.remove(rel) - if history: - ifcopenshell.util.element.remove_deep2(file, history) + if rel.RelatingProcess != relating_process: + continue + if sequence_type is not None and rel.SequenceType != sequence_type: + continue + history = rel.OwnerHistory + file.remove(rel) + if history: + ifcopenshell.util.element.remove_deep2(file, history) ifcopenshell.api.sequence.cascade_schedule(file, task=related_process) diff --git a/src/ifcopenshell-python/test/api/sequence/test_assign_sequence.py b/src/ifcopenshell-python/test/api/sequence/test_assign_sequence.py new file mode 100644 index 0000000000..fa60a1c94a --- /dev/null +++ b/src/ifcopenshell-python/test/api/sequence/test_assign_sequence.py @@ -0,0 +1,95 @@ +# IfcOpenShell - IFC toolkit and geometry engine +# Copyright (C) 2021 Dion Moult +# +# This file is part of IfcOpenShell. +# +# IfcOpenShell is free software: you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# IfcOpenShell is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with IfcOpenShell. If not, see . + +import ifcopenshell.api.sequence +import test.bootstrap + + +# NOTE: sequence module features rely on entities introduced in IFC4 +# therefore no IFC2X3 tests +class TestAssignSequence(test.bootstrap.IFC4): + def test_assigning_a_sequence(self): + predecessor = ifcopenshell.api.sequence.add_task(self.file) + successor = ifcopenshell.api.sequence.add_task(self.file) + rel = ifcopenshell.api.sequence.assign_sequence( + self.file, relating_process=predecessor, related_process=successor + ) + assert rel.is_a("IfcRelSequence") + assert rel.RelatingProcess == predecessor + assert rel.RelatedProcess == successor + assert rel.SequenceType == "FINISH_START" + + def test_assigning_a_sequence_of_a_chosen_type(self): + predecessor = ifcopenshell.api.sequence.add_task(self.file) + successor = ifcopenshell.api.sequence.add_task(self.file) + rel = ifcopenshell.api.sequence.assign_sequence( + self.file, + relating_process=predecessor, + related_process=successor, + sequence_type="START_START", + ) + assert rel.SequenceType == "START_START" + + def test_not_assigning_the_same_sequence_twice(self): + predecessor = ifcopenshell.api.sequence.add_task(self.file) + successor = ifcopenshell.api.sequence.add_task(self.file) + rel1 = ifcopenshell.api.sequence.assign_sequence( + self.file, relating_process=predecessor, related_process=successor + ) + rel2 = ifcopenshell.api.sequence.assign_sequence( + self.file, relating_process=predecessor, related_process=successor + ) + assert rel1 == rel2 + assert len(self.file.by_type("IfcRelSequence")) == 1 + + def test_assigning_two_sequences_of_different_types_to_the_same_pair(self): + # A "ladder": the follower may start once the leader has started, and + # may not finish before the leader finishes. Both constraints are real + # and neither implies the other, so both relationships must survive. + predecessor = ifcopenshell.api.sequence.add_task(self.file) + successor = ifcopenshell.api.sequence.add_task(self.file) + start = ifcopenshell.api.sequence.assign_sequence( + self.file, + relating_process=predecessor, + related_process=successor, + sequence_type="START_START", + ) + finish = ifcopenshell.api.sequence.assign_sequence( + self.file, + relating_process=predecessor, + related_process=successor, + sequence_type="FINISH_FINISH", + ) + assert start != finish + assert len(self.file.by_type("IfcRelSequence")) == 2 + assert {rel.SequenceType for rel in successor.IsSuccessorFrom} == { + "START_START", + "FINISH_FINISH", + } + + def test_not_confusing_the_two_directions_of_a_pair(self): + task1 = ifcopenshell.api.sequence.add_task(self.file) + task2 = ifcopenshell.api.sequence.add_task(self.file) + forwards = ifcopenshell.api.sequence.assign_sequence( + self.file, relating_process=task1, related_process=task2 + ) + backwards = ifcopenshell.api.sequence.assign_sequence( + self.file, relating_process=task2, related_process=task1 + ) + assert forwards != backwards + assert len(self.file.by_type("IfcRelSequence")) == 2 diff --git a/src/ifcopenshell-python/test/api/sequence/test_unassign_sequence.py b/src/ifcopenshell-python/test/api/sequence/test_unassign_sequence.py new file mode 100644 index 0000000000..3a4a6b026d --- /dev/null +++ b/src/ifcopenshell-python/test/api/sequence/test_unassign_sequence.py @@ -0,0 +1,111 @@ +# IfcOpenShell - IFC toolkit and geometry engine +# Copyright (C) 2021 Dion Moult +# +# This file is part of IfcOpenShell. +# +# IfcOpenShell is free software: you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# IfcOpenShell is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with IfcOpenShell. If not, see . + +import ifcopenshell.api.sequence +import test.bootstrap + + +# NOTE: sequence module features rely on entities introduced in IFC4 +# therefore no IFC2X3 tests +class TestUnassignSequence(test.bootstrap.IFC4): + def _pair(self): + return ( + ifcopenshell.api.sequence.add_task(self.file), + ifcopenshell.api.sequence.add_task(self.file), + ) + + def test_unassigning_a_sequence(self): + predecessor, successor = self._pair() + ifcopenshell.api.sequence.assign_sequence( + self.file, relating_process=predecessor, related_process=successor + ) + ifcopenshell.api.sequence.unassign_sequence( + self.file, relating_process=predecessor, related_process=successor + ) + assert len(self.file.by_type("IfcRelSequence")) == 0 + + def test_doing_nothing_if_the_tasks_are_not_sequenced(self): + predecessor, successor = self._pair() + ifcopenshell.api.sequence.unassign_sequence( + self.file, relating_process=predecessor, related_process=successor + ) + assert len(self.file.by_type("IfcRelSequence")) == 0 + + def test_unassigning_every_sequence_between_the_pair_by_default(self): + predecessor, successor = self._pair() + for sequence_type in ("START_START", "FINISH_FINISH"): + ifcopenshell.api.sequence.assign_sequence( + self.file, + relating_process=predecessor, + related_process=successor, + sequence_type=sequence_type, + ) + ifcopenshell.api.sequence.unassign_sequence( + self.file, relating_process=predecessor, related_process=successor + ) + assert len(self.file.by_type("IfcRelSequence")) == 0 + + def test_unassigning_only_the_named_type(self): + predecessor, successor = self._pair() + for sequence_type in ("START_START", "FINISH_FINISH"): + ifcopenshell.api.sequence.assign_sequence( + self.file, + relating_process=predecessor, + related_process=successor, + sequence_type=sequence_type, + ) + ifcopenshell.api.sequence.unassign_sequence( + self.file, + relating_process=predecessor, + related_process=successor, + sequence_type="START_START", + ) + rels = self.file.by_type("IfcRelSequence") + assert len(rels) == 1 + assert rels[0].SequenceType == "FINISH_FINISH" + + def test_not_unassigning_a_type_that_is_not_there(self): + predecessor, successor = self._pair() + ifcopenshell.api.sequence.assign_sequence( + self.file, + relating_process=predecessor, + related_process=successor, + sequence_type="START_START", + ) + ifcopenshell.api.sequence.unassign_sequence( + self.file, + relating_process=predecessor, + related_process=successor, + sequence_type="FINISH_FINISH", + ) + assert len(self.file.by_type("IfcRelSequence")) == 1 + + def test_leaving_the_other_direction_alone(self): + task1, task2 = self._pair() + ifcopenshell.api.sequence.assign_sequence( + self.file, relating_process=task1, related_process=task2 + ) + ifcopenshell.api.sequence.assign_sequence( + self.file, relating_process=task2, related_process=task1 + ) + ifcopenshell.api.sequence.unassign_sequence( + self.file, relating_process=task1, related_process=task2 + ) + rels = self.file.by_type("IfcRelSequence") + assert len(rels) == 1 + assert rels[0].RelatingProcess == task2