From fa895b9683972504b3c5150affba101321e7a88a Mon Sep 17 00:00:00 2001 From: Colin Wood Date: Fri, 8 May 2026 15:08:28 -0700 Subject: [PATCH 1/9] first draft of transitive search and multiple transformer composition --- src/rachis/core/transform.py | 179 +++++++++++++++++++++++++++++++++++ 1 file changed, 179 insertions(+) diff --git a/src/rachis/core/transform.py b/src/rachis/core/transform.py index 30edca41c..432d94f7c 100644 --- a/src/rachis/core/transform.py +++ b/src/rachis/core/transform.py @@ -5,6 +5,9 @@ # # The full license is in the file LICENSE, distributed with this software. # ---------------------------------------------------------------------------- +from __future__ import annotations + +from enum import Enum import pathlib from rachis import sdk @@ -12,6 +15,7 @@ from rachis.core import util + def identity_transformer(view): return view @@ -232,3 +236,178 @@ def validate(self, view, level=None): if not isinstance(view, self._view_type): raise TypeError("%r is not of type %r, cannot transform further." % (view, self._view_type)) + + +class TransformType(Enum): + registered = 1 + wrap = 2 + unwrap = 3 + + +class SearchNode: + def __init__( + self, + type_: type, + parent: SearchNode | None, + transform_type: TransformType = TransformType.registered + ): + self.type_ = type_ + self.parent = parent + self.transform_type = transform_type + + def __eq__(self, other): + return self.type_ == other.type_ + + def __hash__(self): + return hash(self.type_) + + def __repr__(self): + return ( + f'SearchNode(id={id(self)}, type_={repr(self.type_)}, ' + f'parent={None if self.parent is None else id(self.parent)}, ' + f'transform_type={self.transform_type})' + ) + + +def find_transformation_path(start: type, target: type) -> SearchNode | None: + ''' + Searches for a transformation path from `start` to `target`. The path is + encoded in the chain of parents of the returned SearchNode. + + Parameters + ---------- + start : type + The type we wish to transform from. + target : type + The type we wish to transform to. + + Returns + ------- + SearchNode | None + A SearchNode of the target type, if reachable, otherwise None. + ''' + pm = sdk.PluginManager() + + visited: set[SearchNode] = set() + current = SearchNode(type_=start, parent=None) + outstanding: list[SearchNode] = [current] + while outstanding != []: + current = outstanding.pop() + + if current in visited: + continue + else: + visited.add(current) + + if current.type_ == target: + return current + + for neighbor in pm.transformers.get(current.type_, []): + outstanding.insert(0, SearchNode(type_=neighbor, parent=current)) + + if issubclass(current.type_, model.base.FormatBase): + # add synthetic link for Dx -> x + if issubclass(current.type_, model.SingleFileDirectoryFormatBase): + neighbor = SearchNode( + type_=current.type_.file.format, + parent=current, + transform_type=TransformType.unwrap + ) + outstanding.insert(0, neighbor) + + # add synthetic link(s) x -> Dx + else: + for sfdf in pm._ff_to_sfdf.get(current.type_, []): + neighbor = SearchNode( + type_=sfdf, + parent=current, + transform_type=TransformType.wrap + ) + outstanding.insert(0, neighbor) + + return None + + +def compose_transformation(target: SearchNode | None, recorder=None): + if target is None: + return None + + pm = sdk.PluginManager() + + steps = [] + current = target + while current is not None: + steps.insert(0, current) + current = current.parent + + if recorder is not None: + if len(steps) == 1: + # todo (identity case) + pass + for step in steps: + # todo + pass + + if len(steps) == 1: + def identity_transformation(view, validate_level='min'): + from_mt = ModelType.from_view_type(steps[0].type_) + current = from_mt.coerce_view(view) + from_mt.validate(current, level=validate_level) + + return current + + return identity_transformation + + def transformation(view, validate_level='min'): + current = view + for i in range(len(steps) - 1): + from_type = steps[i].type_ + to_type = steps[i + 1].type_ + + if steps[i + 1].transform_type == TransformType.wrap: + transformer = wrap_transformer(from_type, to_type) + elif steps[i + 1].transform_type == TransformType.unwrap: + transformer = unwrap_transformer(from_type) + else: + transformer = pm.transformers[from_type][to_type].transformer + + from_mt = ModelType.from_view_type(from_type) + to_mt = ModelType.from_view_type(to_type) + + current = from_mt.coerce_view(current) + from_mt.validate(current, level=validate_level) + current = transformer(current) + + current = to_mt.coerce_view(current) + to_mt.validate(current, level=validate_level) + to_mt.set_user_owned(current, False) + + return current + + return transformation + + +def wrap_transformer(file_type: type, sfdf_type: type): + ''' + A transformer used to convert any `FileFormat` into its associated + `SingleFileDirectoryFormat`. + ''' + def transformer(view): + sfdf = sfdf_type() + sfdf.file.write_data(view, file_type) + return sfdf + + return transformer + + +def unwrap_transformer(sfdf_type: type): + ''' + A transformer used to convert any `SingleFileDirectoryFormat` into the + contained `FileFormat`. + ''' + file_type = sfdf_type.file.format + + def transformer(view): + return view.file.view(file_type) + + return transformer From 19b43d092c41b50a31a9a898eab7865997c485c1 Mon Sep 17 00:00:00 2001 From: Macabe Wood Date: Tue, 28 Jul 2026 12:07:26 -0700 Subject: [PATCH 2/9] Transitive transformers work(citations don't) --- src/rachis/core/testing/format.py | 20 +++++++ src/rachis/core/testing/plugin.py | 38 +++++++++++- src/rachis/core/testing/transformer.py | 24 +++++++- src/rachis/core/testing/type.py | 5 ++ src/rachis/core/tests/test_transform.py | 54 +++++++++++++++++ src/rachis/core/transform.py | 66 ++++++++++----------- src/rachis/plugin/plugin.py | 20 ++++--- src/rachis/sdk/result.py | 4 -- src/rachis/sdk/tests/test_artifact.py | 7 +-- src/rachis/sdk/tests/test_plugin_manager.py | 19 +++++- 10 files changed, 203 insertions(+), 54 deletions(-) create mode 100644 src/rachis/core/tests/test_transform.py diff --git a/src/rachis/core/testing/format.py b/src/rachis/core/testing/format.py index ecc34dc4c..148481e8e 100644 --- a/src/rachis/core/testing/format.py +++ b/src/rachis/core/testing/format.py @@ -171,3 +171,23 @@ class ExportableOnlyFormat(TextFileFormat): """ A format that can only be transformed to. """ + +class FirstStepFormat(model.DirectoryFormat): + """ + A format for testing transitive transformers + """ + +class SecondStepFormat(model.DirectoryFormat): + """ + A format for testing transitive transformers + """ + +class ThirdStepFormat(model.DirectoryFormat): + """ + A format for testing transitive transformers + """ + +class FourthStepFormat(model.DirectoryFormat): + """ + A format for testing transitive transformers + """ diff --git a/src/rachis/core/testing/plugin.py b/src/rachis/core/testing/plugin.py index 91ac0cb2e..0305c784c 100644 --- a/src/rachis/core/testing/plugin.py +++ b/src/rachis/core/testing/plugin.py @@ -30,12 +30,17 @@ Cephalapod, CephalapodDirectoryFormat, ImportableOnlyFormat, - ExportableOnlyFormat + ExportableOnlyFormat, + FirstStepFormat, + SecondStepFormat, + ThirdStepFormat, + FourthStepFormat ) from .type import (IntSequence1, IntSequence2, IntSequence3, Mapping, FourInts, SingleInt, Kennel, Dog, Cat, C1, C2, C3, Foo, Bar, Baz, - AscIntSequence, Squid, Octopus, Cuttlefish) + AscIntSequence, Squid, Octopus, Cuttlefish, FirstStep, + SecondStep, ThirdStep, FourthStep) from .method import (concatenate_ints, split_ints, merge_mappings, identity_with_metadata, identity_with_metadata_column, identity_with_categorical_metadata_column, @@ -120,7 +125,9 @@ IntSequenceFormatV2, MappingFormat, IntSequenceV2DirectoryFormat, IntSequenceMultiFileDirectoryFormat, MappingDirectoryFormat, EchoDirectoryFormat, EchoFormat, Cephalapod, CephalapodDirectoryFormat, - ImportableOnlyFormat, ExportableOnlyFormat) + ImportableOnlyFormat, ExportableOnlyFormat, FirstStepFormat, + SecondStepFormat, ThirdStepFormat, FourthStepFormat +) dummy_plugin.register_formats( FourIntsDirectoryFormat, UnimportableDirectoryFormat, UnimportableFormat, @@ -182,6 +189,31 @@ def factory(): description="The second IntSequence", examples={'IntSequence2 import example': is2_use} ) + +dummy_plugin.register_artifact_class( + FirstStep, + directory_format=FirstStepFormat, + description="First step" +) + +dummy_plugin.register_artifact_class( + SecondStep, + directory_format=SecondStepFormat, + description="Second step" +) + +dummy_plugin.register_artifact_class( + ThirdStep, + directory_format=ThirdStepFormat, + description="Third step" +) + +dummy_plugin.register_artifact_class( + FourthStep, + directory_format=FourthStepFormat, + description="Fourth step" +) + dummy_plugin.register_semantic_type_to_format( IntSequence3, directory_format=IntSequenceMultiFileDirectoryFormat diff --git a/src/rachis/core/testing/transformer.py b/src/rachis/core/testing/transformer.py index f3e039cd2..4dbddea4a 100644 --- a/src/rachis/core/testing/transformer.py +++ b/src/rachis/core/testing/transformer.py @@ -23,7 +23,11 @@ RedundantSingleIntDirectoryFormat, EchoFormat, ImportableOnlyFormat, - ExportableOnlyFormat + ExportableOnlyFormat, + FirstStepFormat, + SecondStepFormat, + ThirdStepFormat, + FourthStepFormat ) from .plugin import dummy_plugin, citations @@ -206,3 +210,21 @@ def _4242(data: ImportableOnlyFormat) -> IntSequenceDirectoryFormat: @dummy_plugin.register_transformer() def _4243(data: IntSequenceDirectoryFormat) -> ExportableOnlyFormat: return ExportableOnlyFormat() + + +# only for testing transitive transformers +@dummy_plugin.register_transformer(upgrade=True) +def _6000(data: FirstStepFormat) -> SecondStepFormat: + return SecondStepFormat() + + +# only for testing transitive transformers +@dummy_plugin.register_transformer(upgrade=True) +def _6001(data: SecondStepFormat) -> ThirdStepFormat: + return ThirdStepFormat() + + +# only for testing transitive transformers +@dummy_plugin.register_transformer +def _6002(data: ThirdStepFormat) -> FourthStepFormat: + return FourthStepFormat() diff --git a/src/rachis/core/testing/type.py b/src/rachis/core/testing/type.py index 19ce2bcd9..b87a63812 100644 --- a/src/rachis/core/testing/type.py +++ b/src/rachis/core/testing/type.py @@ -45,3 +45,8 @@ Squid = plugin.SemanticType('Squid') Octopus = plugin.SemanticType('Octopus') Cuttlefish = plugin.SemanticType('Cuttlefish') + +FirstStep = plugin.SemanticType('FirstStep') +SecondStep = plugin.SemanticType('SecondStep') +ThirdStep = plugin.SemanticType('ThirdStep') +FourthStep = plugin.SemanticType('FourthStep') diff --git a/src/rachis/core/tests/test_transform.py b/src/rachis/core/tests/test_transform.py new file mode 100644 index 000000000..e0adeeee5 --- /dev/null +++ b/src/rachis/core/tests/test_transform.py @@ -0,0 +1,54 @@ +import unittest +from typing import Union +from tempfile import TemporaryDirectory + +from rachis import Artifact +from rachis.sdk import PluginManager +from rachis.core.testing.format import ( + ThirdStepFormat, FourthStepFormat, Cephalapod +) + + +class TestTransitiveTransfomrers(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.pm = PluginManager() + with TemporaryDirectory() as tempdir: + cls.first_format = Artifact.import_data( + type='FirstStep', view=tempdir + ) + cls.int_sequence = Artifact.import_data( + type='IntSequence1', view=[1, 2, 3] + ) + + def test_first_to_third(self): + """ + Path exists and is upgraded. + FirstStepFormat -> SecondStepFormat -> ThirdStepFormat + """ + view = self.first_format.view(ThirdStepFormat) + self.assertEqual(type(view), ThirdStepFormat) + + def test_first_to_fourth_fails(self): + """ + Path exists but is not upgraded. + FirstStepFormat -> SecondStepFormat -> ThirdStepFormat x-> Fourth + """ + with self.assertRaisesRegex(Exception, 'No transformation from'): + self.first_format.view(FourthStepFormat) + + def test_union_transitivity(self): + """ + Path exists between FirstStepFormat and ThirdStepFormat but not between + FirsStepFormat and Cephalapod. + """ + view = self.first_format.view(Union[Cephalapod, ThirdStepFormat]) + self.assertEqual(type(view), ThirdStepFormat) + + def test_int_sequence_dir_to_list(self): + """ + Tests that unwrapping a format and then transforming does not require + upgrading the path. + """ + view = self.int_sequence.view(list) + self.assertEqual(type(view), list) diff --git a/src/rachis/core/transform.py b/src/rachis/core/transform.py index 432d94f7c..1cba61330 100644 --- a/src/rachis/core/transform.py +++ b/src/rachis/core/transform.py @@ -15,7 +15,6 @@ from rachis.core import util - def identity_transformer(view): return view @@ -56,32 +55,15 @@ def __init__(self, view_type): self._record = self._pm.views[self._view_name] def make_transformation(self, other, recorder=None): - # record may be None in case of identity transformer - transformer, transformer_record = self._get_transformer_to(other) - if transformer is None: + target_node = find_transformation_path( + self._view_type, other._view_type + ) + + if target_node is None: raise Exception("No transformation from %r to %r" % (self._view_type, other._view_type)) - if recorder is not None: - recorder(transformer_record, input_name=self._view_name, - input_record=self._record, output_name=other._view_name, - output_record=other._record) - - def transformation(view, validate_level='min'): - view = self.coerce_view(view) - self.validate(view, level=validate_level) - - new_view = transformer(view) - - new_view = other.coerce_view(new_view) - other.validate(new_view) - - if transformer is not identity_transformer: - other.set_user_owned(new_view, False) - - return new_view - - return transformation + return compose_transformation(target_node) def _get_transformer_to(self, other): transformer, record = self._lookup_transformer(self._view_type, @@ -117,11 +99,11 @@ def coerce_view(self, view): def _lookup_transformer(self, from_, to_): if from_ == to_: return identity_transformer, None - try: - record = self._pm.transformers[from_][to_] - return record.transformer, record - except KeyError: + + search_node = find_transformation_path(from_, to_) + if search_node is None or search_node.record is None: return None, None + return search_node.record.transformer, search_node.record def set_user_owned(self, view, value): pass @@ -249,11 +231,15 @@ def __init__( self, type_: type, parent: SearchNode | None, - transform_type: TransformType = TransformType.registered + record = None, + transform_type: TransformType = TransformType.registered, + steps = 0 ): self.type_ = type_ self.parent = parent + self.record = record self.transform_type = transform_type + self.steps = steps def __eq__(self, other): return self.type_ == other.type_ @@ -302,8 +288,18 @@ def find_transformation_path(start: type, target: type) -> SearchNode | None: if current.type_ == target: return current - for neighbor in pm.transformers.get(current.type_, []): - outstanding.insert(0, SearchNode(type_=neighbor, parent=current)) + for neighbor, transform_record in pm.transformers.get( + current.type_, {} + ).items(): + if transform_record.upgrade or current.steps == 0: + outstanding.insert( + 0, SearchNode( + type_=neighbor, + parent=current, + record=transform_record, + steps=current.steps+1 + ) + ) if issubclass(current.type_, model.base.FormatBase): # add synthetic link for Dx -> x @@ -311,7 +307,9 @@ def find_transformation_path(start: type, target: type) -> SearchNode | None: neighbor = SearchNode( type_=current.type_.file.format, parent=current, - transform_type=TransformType.unwrap + record=current.record, + transform_type=TransformType.unwrap, + steps=current.steps ) outstanding.insert(0, neighbor) @@ -321,7 +319,9 @@ def find_transformation_path(start: type, target: type) -> SearchNode | None: neighbor = SearchNode( type_=sfdf, parent=current, - transform_type=TransformType.wrap + record=current.record, + transform_type=TransformType.wrap, + steps=current.steps ) outstanding.insert(0, neighbor) diff --git a/src/rachis/plugin/plugin.py b/src/rachis/plugin/plugin.py index 6052606f1..ea4b7490d 100644 --- a/src/rachis/plugin/plugin.py +++ b/src/rachis/plugin/plugin.py @@ -74,7 +74,7 @@ TransformerRecord = collections.namedtuple( - 'TransformerRecord', ['transformer', 'plugin', 'citations']) + 'TransformerRecord', ['transformer', 'plugin', 'citations', 'upgrade']) SemanticTypeRecord = collections.namedtuple( 'SemanticTypeRecord', ['semantic_type', 'plugin']) SemanticTypeFragmentRecord = collections.namedtuple( @@ -341,11 +341,9 @@ def decorator(validator): return validator return decorator - def register_transformer(self, _fn=None, *, citations=None): + def register_transformer(self, _fn=None, *, citations=None, upgrade=False): """ **Decorator** which registers a transformer to convert data - This decorator may be used with or without arguments. - Parameters ---------- _fn : Callable @@ -354,7 +352,11 @@ def register_transformer(self, _fn=None, *, citations=None): citations : CitationRecord or list of CitationRecord Citation(s) to associate with a result whenever this transformer is used internally. Can also use an entire :py:class:`Citations` object. - + upgrade : Bool + Whether to include this transformer when searching for paths + between transformers. This is decided based on whether the + transformer loses important information when converting from one + type to the other. Returns ------- decorator @@ -372,7 +374,7 @@ def register_transformer(self, _fn=None, *, citations=None): Examples -------- - >>> @plugin.register_transformer + >>> @plugin.register_transformer(upgrade=True) ... def _0(data: pd.DataFrame) -> CSVFormat: ... ff = CSVFormat() ... with ff.open() as fh: @@ -426,7 +428,11 @@ def decorator(transformer): % (transformer, input, output)) self.transformers[input, output] = TransformerRecord( - transformer=transformer, plugin=self, citations=citations) + transformer=transformer, + plugin=self, + citations=citations, + upgrade=upgrade + ) return transformer if _fn is None: diff --git a/src/rachis/sdk/result.py b/src/rachis/sdk/result.py index 29241a42d..4900371c3 100644 --- a/src/rachis/sdk/result.py +++ b/src/rachis/sdk/result.py @@ -461,10 +461,6 @@ def view(self, view_type): return self._view(view_type) def _view(self, view_type, recorder=None): - if view_type is rachis.Metadata and not self.has_metadata(): - raise TypeError( - "Artifact %r cannot be viewed as Rachis Metadata." % self) - from_type = transform.ModelType.from_view_type(self.format) if isinstance(get_origin(view_type), type(Union)): diff --git a/src/rachis/sdk/tests/test_artifact.py b/src/rachis/sdk/tests/test_artifact.py index d6356a8b4..b8ca02a93 100644 --- a/src/rachis/sdk/tests/test_artifact.py +++ b/src/rachis/sdk/tests/test_artifact.py @@ -563,9 +563,8 @@ def test_artifact_validate_max(self): self.assertTrue(True) # Checkpoint assertion A.validate(level='max') self.assertTrue(True) # Checkpoint assertion - A = Artifact.import_data('IntSequence1', [1, 2, 3, 4, 5, 6, 7, 10]) with self.assertRaisesRegex(ValidationError, '3 more'): - A.validate(level='max') + A = Artifact.import_data('IntSequence1', [1, 2, 3, 4, 5, 6, 7, 10]) def test_artifact_validate_max_on_import(self): fp = get_data_path('intsequence-fail-max-validation.txt') @@ -610,9 +609,7 @@ def test_view_as_metadata(self): def test_cannot_be_viewed_as_metadata(self): A = Artifact.import_data('IntSequence1', [1, 2, 3, 4]) - with self.assertRaisesRegex(TypeError, - 'Artifact.*IntSequence1.*cannot be viewed ' - 'as Rachis Metadata'): + with self.assertRaisesRegex(Exception, 'No transformation from '): A.view(Metadata) diff --git a/src/rachis/sdk/tests/test_plugin_manager.py b/src/rachis/sdk/tests/test_plugin_manager.py index f0450fec6..53fcb2bbd 100644 --- a/src/rachis/sdk/tests/test_plugin_manager.py +++ b/src/rachis/sdk/tests/test_plugin_manager.py @@ -32,7 +32,12 @@ EchoDirectoryFormat, CephalapodDirectoryFormat, ImportableOnlyFormat, - ExportableOnlyFormat) + ExportableOnlyFormat, + FirstStepFormat, + SecondStepFormat, + ThirdStepFormat, + FourthStepFormat +) from rachis.core.testing.validator import (validator_example_null1, validate_ascending_seq, @@ -221,6 +226,18 @@ def test_get_formats_no_type_or_filter(self): 'ExportableOnlyFormat': FormatRecord(format=ExportableOnlyFormat, plugin=self.plugin), + 'FirstStepFormat': + FormatRecord(format=FirstStepFormat, + plugin=self.plugin), + 'SecondStepFormat': + FormatRecord(format=SecondStepFormat, + plugin=self.plugin), + 'ThirdStepFormat': + FormatRecord(format=ThirdStepFormat, + plugin=self.plugin), + 'FourthStepFormat': + FormatRecord(format=FourthStepFormat, + plugin=self.plugin), } obs = self.pm.get_formats() From 3ae676465f61c07c433a6fdb04807a18abf378ab Mon Sep 17 00:00:00 2001 From: Macabe Wood Date: Tue, 28 Jul 2026 13:52:04 -0700 Subject: [PATCH 3/9] recorder --- src/rachis/core/transform.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/rachis/core/transform.py b/src/rachis/core/transform.py index 1cba61330..a4ab7add7 100644 --- a/src/rachis/core/transform.py +++ b/src/rachis/core/transform.py @@ -63,6 +63,15 @@ def make_transformation(self, other, recorder=None): raise Exception("No transformation from %r to %r" % (self._view_type, other._view_type)) + if recorder is not None: + recorder( + target_node.record, + input_name=self._view_name, + input_record=self._record, + output_name=other._view_name, + output_record=other._record + ) + return compose_transformation(target_node) def _get_transformer_to(self, other): @@ -328,7 +337,7 @@ def find_transformation_path(start: type, target: type) -> SearchNode | None: return None -def compose_transformation(target: SearchNode | None, recorder=None): +def compose_transformation(target: SearchNode | None): if target is None: return None @@ -340,14 +349,6 @@ def compose_transformation(target: SearchNode | None, recorder=None): steps.insert(0, current) current = current.parent - if recorder is not None: - if len(steps) == 1: - # todo (identity case) - pass - for step in steps: - # todo - pass - if len(steps) == 1: def identity_transformation(view, validate_level='min'): from_mt = ModelType.from_view_type(steps[0].type_) From b3628e53ca5e2c6d43fb0187922f46132d2c2567 Mon Sep 17 00:00:00 2001 From: Macabe Wood Date: Mon, 3 Aug 2026 15:43:47 -0700 Subject: [PATCH 4/9] adding recorder --- .../core/archive/tests/test_citations.py | 5 +- src/rachis/core/testing/format.py | 5 + src/rachis/core/testing/plugin.py | 13 +- src/rachis/core/testing/transformer.py | 9 +- src/rachis/core/testing/type.py | 1 + src/rachis/core/tests/test_transform.py | 16 +- src/rachis/core/transform.py | 176 ++++++++++++------ src/rachis/plugin/plugin.py | 10 +- src/rachis/sdk/tests/test_plugin_manager.py | 6 +- 9 files changed, 169 insertions(+), 72 deletions(-) diff --git a/src/rachis/core/archive/tests/test_citations.py b/src/rachis/core/archive/tests/test_citations.py index 3ab3b403b..c53fe660e 100644 --- a/src/rachis/core/archive/tests/test_citations.py +++ b/src/rachis/core/archive/tests/test_citations.py @@ -16,6 +16,7 @@ class TestCitationsTracked(unittest.TestCase): def setUp(self): self.plugin = get_dummy_plugin() + self.maxDiff = None def test_import(self): data = rachis.Artifact.import_data(IntSequence1, [1, 2, 3, 4]) @@ -39,7 +40,7 @@ def test_import(self): obs = list(map(lambda item: (item[0], item[1].fields['title']), archiver.citations.items())) - self.assertEqual(obs, expected) + self.assertEqual(sorted(obs), sorted(expected)) with (archiver.provenance_dir / 'action' / 'action.yaml').open() as fh: action_yaml = fh.read() @@ -77,7 +78,7 @@ def test_action(self): obs = list(map(lambda item: (item[0], item[1].fields['title']), archiver.citations.items())) - self.assertEqual(obs, expected) + self.assertEqual(sorted(obs), sorted(expected)) with (archiver.provenance_dir / 'action' / 'action.yaml').open() as fh: action_yaml = fh.read() diff --git a/src/rachis/core/testing/format.py b/src/rachis/core/testing/format.py index 148481e8e..b36d42fe2 100644 --- a/src/rachis/core/testing/format.py +++ b/src/rachis/core/testing/format.py @@ -191,3 +191,8 @@ class FourthStepFormat(model.DirectoryFormat): """ A format for testing transitive transformers """ + +class FifthStepFormat(model.DirectoryFormat): + """ + A format for testing transitive transformers + """ diff --git a/src/rachis/core/testing/plugin.py b/src/rachis/core/testing/plugin.py index 0305c784c..a53214498 100644 --- a/src/rachis/core/testing/plugin.py +++ b/src/rachis/core/testing/plugin.py @@ -34,13 +34,14 @@ FirstStepFormat, SecondStepFormat, ThirdStepFormat, - FourthStepFormat + FourthStepFormat, + FifthStepFormat, ) from .type import (IntSequence1, IntSequence2, IntSequence3, Mapping, FourInts, SingleInt, Kennel, Dog, Cat, C1, C2, C3, Foo, Bar, Baz, AscIntSequence, Squid, Octopus, Cuttlefish, FirstStep, - SecondStep, ThirdStep, FourthStep) + SecondStep, ThirdStep, FourthStep, FifthStep) from .method import (concatenate_ints, split_ints, merge_mappings, identity_with_metadata, identity_with_metadata_column, identity_with_categorical_metadata_column, @@ -126,7 +127,7 @@ IntSequenceMultiFileDirectoryFormat, MappingDirectoryFormat, EchoDirectoryFormat, EchoFormat, Cephalapod, CephalapodDirectoryFormat, ImportableOnlyFormat, ExportableOnlyFormat, FirstStepFormat, - SecondStepFormat, ThirdStepFormat, FourthStepFormat + SecondStepFormat, ThirdStepFormat, FourthStepFormat, FifthStepFormat, ) dummy_plugin.register_formats( @@ -214,6 +215,12 @@ def factory(): description="Fourth step" ) +dummy_plugin.register_artifact_class( + FifthStep, + directory_format=FifthStepFormat, + description="Fifth step" +) + dummy_plugin.register_semantic_type_to_format( IntSequence3, directory_format=IntSequenceMultiFileDirectoryFormat diff --git a/src/rachis/core/testing/transformer.py b/src/rachis/core/testing/transformer.py index 4dbddea4a..722dc6123 100644 --- a/src/rachis/core/testing/transformer.py +++ b/src/rachis/core/testing/transformer.py @@ -27,7 +27,8 @@ FirstStepFormat, SecondStepFormat, ThirdStepFormat, - FourthStepFormat + FourthStepFormat, + FifthStepFormat, ) from .plugin import dummy_plugin, citations @@ -228,3 +229,9 @@ def _6001(data: SecondStepFormat) -> ThirdStepFormat: @dummy_plugin.register_transformer def _6002(data: ThirdStepFormat) -> FourthStepFormat: return FourthStepFormat() + + +# only for testing transitive transformers +@dummy_plugin.register_transformer(upgrade=False) +def _6003(data: ThirdStepFormat) -> FifthStepFormat: + return FifthStepFormat() diff --git a/src/rachis/core/testing/type.py b/src/rachis/core/testing/type.py index b87a63812..44603e08e 100644 --- a/src/rachis/core/testing/type.py +++ b/src/rachis/core/testing/type.py @@ -50,3 +50,4 @@ SecondStep = plugin.SemanticType('SecondStep') ThirdStep = plugin.SemanticType('ThirdStep') FourthStep = plugin.SemanticType('FourthStep') +FifthStep = plugin.SemanticType('FifthStep') diff --git a/src/rachis/core/tests/test_transform.py b/src/rachis/core/tests/test_transform.py index e0adeeee5..9c276fadc 100644 --- a/src/rachis/core/tests/test_transform.py +++ b/src/rachis/core/tests/test_transform.py @@ -3,20 +3,18 @@ from tempfile import TemporaryDirectory from rachis import Artifact -from rachis.sdk import PluginManager from rachis.core.testing.format import ( - ThirdStepFormat, FourthStepFormat, Cephalapod -) + ThirdStepFormat, FourthStepFormat, FifthStepFormat, Cephalapod) class TestTransitiveTransfomrers(unittest.TestCase): @classmethod def setUpClass(cls): - cls.pm = PluginManager() with TemporaryDirectory() as tempdir: cls.first_format = Artifact.import_data( type='FirstStep', view=tempdir ) + cls.int_sequence = Artifact.import_data( type='IntSequence1', view=[1, 2, 3] ) @@ -32,7 +30,7 @@ def test_first_to_third(self): def test_first_to_fourth_fails(self): """ Path exists but is not upgraded. - FirstStepFormat -> SecondStepFormat -> ThirdStepFormat x-> Fourth + FirstStepFormat -> SecondStepFormat -> ThirdStepFormat -None-> Fourth """ with self.assertRaisesRegex(Exception, 'No transformation from'): self.first_format.view(FourthStepFormat) @@ -52,3 +50,11 @@ def test_int_sequence_dir_to_list(self): """ view = self.int_sequence.view(list) self.assertEqual(type(view), list) + + def test_lossy_transformer(self): + """ + Tests that path with lossy steps still completes. + First -> Second -> Third -lossy-> Fifth + """ + view = self.first_format.view(FifthStepFormat) + self.assertEqual(type(view), FifthStepFormat) diff --git a/src/rachis/core/transform.py b/src/rachis/core/transform.py index a4ab7add7..3556503e4 100644 --- a/src/rachis/core/transform.py +++ b/src/rachis/core/transform.py @@ -63,16 +63,16 @@ def make_transformation(self, other, recorder=None): raise Exception("No transformation from %r to %r" % (self._view_type, other._view_type)) - if recorder is not None: - recorder( - target_node.record, - input_name=self._view_name, - input_record=self._record, - output_name=other._view_name, - output_record=other._record - ) + # if recorder is not None: + # recorder( + # target_node.record, + # input_name=self._view_name, + # input_record=self._record, + # output_name=other._view_name, + # output_record=other._record + # ) - return compose_transformation(target_node) + return compose_transformation(target_node, recorder=recorder) def _get_transformer_to(self, other): transformer, record = self._lookup_transformer(self._view_type, @@ -242,13 +242,15 @@ def __init__( parent: SearchNode | None, record = None, transform_type: TransformType = TransformType.registered, - steps = 0 + steps = 0, + wrapped = False ): self.type_ = type_ self.parent = parent self.record = record self.transform_type = transform_type self.steps = steps + self.wrapped = wrapped def __eq__(self, other): return self.type_ == other.type_ @@ -264,6 +266,55 @@ def __repr__(self): ) +def insert_neighbors( + node: SearchNode, outstanding: list[SearchNode], allow_lossy: bool = False +): + pm = sdk.PluginManager() + + for neighbor, transform_record in pm.transformers.get( + node.type_, {} + ).items(): + allowed = ( + transform_record.upgrade is not None if allow_lossy + else transform_record.upgrade + ) + if allowed or node.steps == 0: + outstanding.insert( + 0, SearchNode( + type_=neighbor, + parent=node, + record=transform_record, + steps=node.steps+1 + ) + ) + + if issubclass(node.type_, model.base.FormatBase): + # add synthetic link for Dx -> x + if issubclass(node.type_, model.SingleFileDirectoryFormatBase): + neighbor = SearchNode( + type_=node.type_.file.format, + parent=node, + record=None, + transform_type=TransformType.unwrap, + steps=node.steps, + ) + node.wrapped = True + outstanding.insert(0, neighbor) + + # add synthetic link(s) x -> Dx + else: + for sfdf in pm._ff_to_sfdf.get(node.type_, []): + neighbor = SearchNode( + type_=sfdf, + parent=node, + record=None, + transform_type=TransformType.wrap, + steps=node.steps + ) + node.wrapped = True + outstanding.insert(0, neighbor) + + def find_transformation_path(start: type, target: type) -> SearchNode | None: ''' Searches for a transformation path from `start` to `target`. The path is @@ -281,63 +332,42 @@ def find_transformation_path(start: type, target: type) -> SearchNode | None: SearchNode | None A SearchNode of the target type, if reachable, otherwise None. ''' - pm = sdk.PluginManager() - visited: set[SearchNode] = set() + lossy_visited: set[SearchNode] = set() current = SearchNode(type_=start, parent=None) outstanding: list[SearchNode] = [current] - while outstanding != []: - current = outstanding.pop() + lossy_outstanding: list[SearchNode] = [current] + lossy_found = None - if current in visited: + while outstanding or lossy_outstanding: + current = outstanding.pop() if outstanding else None + lossy_current = lossy_outstanding.pop() if lossy_outstanding else None + + if current in visited and lossy_current in lossy_visited: continue - else: + if current is not None: visited.add(current) + if lossy_current is not None: + lossy_visited.add(lossy_current) + + if current is not None: + if current.type_ == target: + return current + if lossy_current is not None: + if lossy_current.type_ == target: + lossy_found = lossy_current + + if current is not None: + insert_neighbors(current, outstanding) + if lossy_current is not None: + insert_neighbors( + lossy_current, lossy_outstanding, allow_lossy=True + ) - if current.type_ == target: - return current - - for neighbor, transform_record in pm.transformers.get( - current.type_, {} - ).items(): - if transform_record.upgrade or current.steps == 0: - outstanding.insert( - 0, SearchNode( - type_=neighbor, - parent=current, - record=transform_record, - steps=current.steps+1 - ) - ) - - if issubclass(current.type_, model.base.FormatBase): - # add synthetic link for Dx -> x - if issubclass(current.type_, model.SingleFileDirectoryFormatBase): - neighbor = SearchNode( - type_=current.type_.file.format, - parent=current, - record=current.record, - transform_type=TransformType.unwrap, - steps=current.steps - ) - outstanding.insert(0, neighbor) - - # add synthetic link(s) x -> Dx - else: - for sfdf in pm._ff_to_sfdf.get(current.type_, []): - neighbor = SearchNode( - type_=sfdf, - parent=current, - record=current.record, - transform_type=TransformType.wrap, - steps=current.steps - ) - outstanding.insert(0, neighbor) - - return None + return lossy_found -def compose_transformation(target: SearchNode | None): +def compose_transformation(target: SearchNode | None, recorder = None): if target is None: return None @@ -349,6 +379,36 @@ def compose_transformation(target: SearchNode | None): steps.insert(0, current) current = current.parent + if recorder is not None: + for i in range(len(steps) - 1): + name = util.get_view_name(steps[i].type_) + view = pm.views.get(name) + if steps[i].wrapped: + continue + elif steps[i + 1].wrapped: + try: + parent_name = util.get_view_name(steps[i + 2].type_) + parent_view = pm.views.get(parent_name) + recorder( + steps[i + 1].record, + name, + view, + parent_name, + parent_view + ) + except IndexError: + pass + else: + parent_name = util.get_view_name(steps[i + 1].type_) + parent_view = pm.views.get(parent_name) + recorder( + steps[i + 1].record, + name, + view, + parent_name, + parent_view + ) + if len(steps) == 1: def identity_transformation(view, validate_level='min'): from_mt = ModelType.from_view_type(steps[0].type_) diff --git a/src/rachis/plugin/plugin.py b/src/rachis/plugin/plugin.py index ea4b7490d..283ad9aea 100644 --- a/src/rachis/plugin/plugin.py +++ b/src/rachis/plugin/plugin.py @@ -341,7 +341,7 @@ def decorator(validator): return validator return decorator - def register_transformer(self, _fn=None, *, citations=None, upgrade=False): + def register_transformer(self, _fn=None, *, citations=None, upgrade=None): """ **Decorator** which registers a transformer to convert data Parameters @@ -352,11 +352,17 @@ def register_transformer(self, _fn=None, *, citations=None, upgrade=False): citations : CitationRecord or list of CitationRecord Citation(s) to associate with a result whenever this transformer is used internally. Can also use an entire :py:class:`Citations` object. - upgrade : Bool + upgrade : Bool | None Whether to include this transformer when searching for paths between transformers. This is decided based on whether the transformer loses important information when converting from one type to the other. + True: Transformer does not loose infromationa and should be + included in searches. + False: Transformer does loose information but can still be used in + searches as a fall back option. + None: Transformer looses too much information and cannot be used in + searches. This is the default. Returns ------- decorator diff --git a/src/rachis/sdk/tests/test_plugin_manager.py b/src/rachis/sdk/tests/test_plugin_manager.py index 53fcb2bbd..659f6e801 100644 --- a/src/rachis/sdk/tests/test_plugin_manager.py +++ b/src/rachis/sdk/tests/test_plugin_manager.py @@ -36,7 +36,8 @@ FirstStepFormat, SecondStepFormat, ThirdStepFormat, - FourthStepFormat + FourthStepFormat, + FifthStepFormat ) from rachis.core.testing.validator import (validator_example_null1, @@ -238,6 +239,9 @@ def test_get_formats_no_type_or_filter(self): 'FourthStepFormat': FormatRecord(format=FourthStepFormat, plugin=self.plugin), + 'FifthStepFormat': + FormatRecord(format=FifthStepFormat, + plugin=self.plugin), } obs = self.pm.get_formats() From b81a6f76195e18f894dfc612d3ecb1bb64966502 Mon Sep 17 00:00:00 2001 From: Macabe Wood Date: Wed, 5 Aug 2026 12:07:56 -0700 Subject: [PATCH 5/9] small --- src/rachis/core/archive/tests/test_citations.py | 1 - src/rachis/core/transform.py | 9 --------- 2 files changed, 10 deletions(-) diff --git a/src/rachis/core/archive/tests/test_citations.py b/src/rachis/core/archive/tests/test_citations.py index c53fe660e..2cd47bf62 100644 --- a/src/rachis/core/archive/tests/test_citations.py +++ b/src/rachis/core/archive/tests/test_citations.py @@ -16,7 +16,6 @@ class TestCitationsTracked(unittest.TestCase): def setUp(self): self.plugin = get_dummy_plugin() - self.maxDiff = None def test_import(self): data = rachis.Artifact.import_data(IntSequence1, [1, 2, 3, 4]) diff --git a/src/rachis/core/transform.py b/src/rachis/core/transform.py index 3556503e4..5ce05ac2b 100644 --- a/src/rachis/core/transform.py +++ b/src/rachis/core/transform.py @@ -63,15 +63,6 @@ def make_transformation(self, other, recorder=None): raise Exception("No transformation from %r to %r" % (self._view_type, other._view_type)) - # if recorder is not None: - # recorder( - # target_node.record, - # input_name=self._view_name, - # input_record=self._record, - # output_name=other._view_name, - # output_record=other._record - # ) - return compose_transformation(target_node, recorder=recorder) def _get_transformer_to(self, other): From 4c07b801156038f3f200d073107df4f4f644f2ee Mon Sep 17 00:00:00 2001 From: Colin Wood <68213641+colinvwood@users.noreply.github.com> Date: Thu, 3 Sep 2026 10:03:41 -0700 Subject: [PATCH 6/9] WIP: docstrings, alg refactor --- src/rachis/core/transform.py | 207 ++++++++++++++++++++++++++--------- src/rachis/plugin/plugin.py | 24 ++-- 2 files changed, 169 insertions(+), 62 deletions(-) diff --git a/src/rachis/core/transform.py b/src/rachis/core/transform.py index 5ce05ac2b..3663cc6c2 100644 --- a/src/rachis/core/transform.py +++ b/src/rachis/core/transform.py @@ -221,28 +221,83 @@ def validate(self, view, level=None): class TransformType(Enum): + ''' + Annotates a `SearchNode` as being transformed from its parent in one of + three ways: + + - `registered` means that the transformation was a typical, registered + transformer + - `wrap` means that the transformation converts a `FormatBase` into a + `SingleFileDirectoryFormatBase` + - `unwrap` type means that the transformation converts a + `SingleFileDirectoryFormatBase` into a `FormatBase` + ''' registered = 1 wrap = 2 unwrap = 3 +class PathType(Enum): + ''' + Annotates the path encoded in a `SearchNode` as belonging to one of three + categories: + - upgrade_only: only implicit and `upgrade=True` transformations + - includes_false: implicit, `upgrade=True`, and `upgrade=False` + transformations + - includes_none: implicit, `upgrade=True`, `upgrade=False` and + `upgrade=None` transformations + ''' + upgrade_only = 1 + includes_false = 2 + includes_none = 3 + + class SearchNode: def __init__( self, type_: type, parent: SearchNode | None, - record = None, + record: 'TransformerRecord' | None = None, transform_type: TransformType = TransformType.registered, - steps = 0, - wrapped = False + wrapped: bool = False ): + ''' + Parameters + ---------- + type_ : type + The type of the node. + parent : SearchNode | None + The node from which this node has been transformed, or None if this + is the first node in the path. + record : TransformerRecord | None + The `TransformerRecord` as registered in `Plugin.transformers` when + the transformation from parent to self was registered, or None for + wrap/unwrap transformations. + transform_type : TransformType + See `TransformType`. + wrapped : bool + + ''' self.type_ = type_ self.parent = parent self.record = record self.transform_type = transform_type - self.steps = steps self.wrapped = wrapped + def __len__(self): + ''' + Returns the number of registered transformers up to this node. + ''' + length = 0 + n = self + while n.parent is not None: + if n.record is not None: + length += 1 + + n = n.parent + + return length + def __eq__(self, other): return self.type_ == other.type_ @@ -257,28 +312,92 @@ def __repr__(self): ) -def insert_neighbors( - node: SearchNode, outstanding: list[SearchNode], allow_lossy: bool = False -): +class NodeQueue: + def __init__(self): + self.nodes = [] + + def push(self, node: SearchNode) -> None: + ''' + Inserts a node and resorts the queue. + + The queue is ordered according to the following algorithm. A + transformer falls into one of three categories: + - implicit transformers (wrap/unwrap transformations to and from + file formats and single file directory formats) + - explicit transformers that are marked `upgrade=True` + - explicit transformers that are marked `upgrade=False` or + `upgrade=None` + + The preference is in the order listed above, with the following + justification. Implicit transformers are the most likely to initiate + or complete the transformation path as we often begin and end with + file formats or single file directory formats. Transformers marked + `upgrade=True` are non-lossy and can be chained indefinitely. + Transformers marked `upgrade=False` or `upgrade=None` are lossy and + should be used only as a last resort. + + Within each category, paths that are shorter are preferred. + ''' + self.nodes.append(node) + + def primary(node): + return int(self.classify_node(node).value) + + def secondary(node): + return len(node) + + self.nodes.sort(key=lambda n: (primary(n), secondary(n)), reverse=True) + + def pop(self) -> SearchNode | None: + if not self.nodes: + return None + + return self.nodes.pop() + + def classify_node(self, node: SearchNode) -> PathType: + ''' + Classify the path encoded in a `SearchNode` as one of `PathType`. + ''' + status = PathType.upgrade_only + while node is not None: + if node.record is not None and node.record.upgrade is False: + status = PathType.includes_false + elif node.record is not None and node.record.upgrade is None: + status = PathType.includes_none + return status + + node = node.parent + + return status + + +def insert_neighbors(node: SearchNode, node_queue: NodeQueue) -> None: + ''' + Find explicit and implicit neighbors to `node` and add them to + `node_queue`. + + Parameters + ---------- + node : SearchNode + The node the neighbors of which should be added. + node_queue : NodeQueue + The remaining nodes to search while finding a transformation path. + Neighbors are pushed into this queue. + ''' pm = sdk.PluginManager() + # explicit neighbors for neighbor, transform_record in pm.transformers.get( node.type_, {} ).items(): - allowed = ( - transform_record.upgrade is not None if allow_lossy - else transform_record.upgrade + neighbor_node = SearchNode( + type_=neighbor, + parent=node, + record=transform_record, ) - if allowed or node.steps == 0: - outstanding.insert( - 0, SearchNode( - type_=neighbor, - parent=node, - record=transform_record, - steps=node.steps+1 - ) - ) + node_queue.push(neighbor_node) + # implicit neighbors if issubclass(node.type_, model.base.FormatBase): # add synthetic link for Dx -> x if issubclass(node.type_, model.SingleFileDirectoryFormatBase): @@ -287,10 +406,9 @@ def insert_neighbors( parent=node, record=None, transform_type=TransformType.unwrap, - steps=node.steps, ) node.wrapped = True - outstanding.insert(0, neighbor) + node_queue.push(neighbor) # add synthetic link(s) x -> Dx else: @@ -300,16 +418,15 @@ def insert_neighbors( parent=node, record=None, transform_type=TransformType.wrap, - steps=node.steps ) node.wrapped = True - outstanding.insert(0, neighbor) + node_queue.push(neighbor) def find_transformation_path(start: type, target: type) -> SearchNode | None: ''' Searches for a transformation path from `start` to `target`. The path is - encoded in the chain of parents of the returned SearchNode. + encoded in the chain of parents of the returned `SearchNode`. Parameters ---------- @@ -323,39 +440,25 @@ def find_transformation_path(start: type, target: type) -> SearchNode | None: SearchNode | None A SearchNode of the target type, if reachable, otherwise None. ''' - visited: set[SearchNode] = set() - lossy_visited: set[SearchNode] = set() current = SearchNode(type_=start, parent=None) - outstanding: list[SearchNode] = [current] - lossy_outstanding: list[SearchNode] = [current] - lossy_found = None + visited: set[SearchNode] = set() - while outstanding or lossy_outstanding: - current = outstanding.pop() if outstanding else None - lossy_current = lossy_outstanding.pop() if lossy_outstanding else None + node_queue = NodeQueue() + node_queue.push(current) + while True: + current = node_queue.pop() - if current in visited and lossy_current in lossy_visited: + if current is None: + return None + + if current in visited: continue - if current is not None: - visited.add(current) - if lossy_current is not None: - lossy_visited.add(lossy_current) - - if current is not None: - if current.type_ == target: - return current - if lossy_current is not None: - if lossy_current.type_ == target: - lossy_found = lossy_current - - if current is not None: - insert_neighbors(current, outstanding) - if lossy_current is not None: - insert_neighbors( - lossy_current, lossy_outstanding, allow_lossy=True - ) - return lossy_found + if current.type_ == target: + return current + + visited.add(current) + insert_neighbors(current, node_queue) def compose_transformation(target: SearchNode | None, recorder = None): diff --git a/src/rachis/plugin/plugin.py b/src/rachis/plugin/plugin.py index 283ad9aea..3e1e098e3 100644 --- a/src/rachis/plugin/plugin.py +++ b/src/rachis/plugin/plugin.py @@ -353,16 +353,20 @@ def register_transformer(self, _fn=None, *, citations=None, upgrade=None): Citation(s) to associate with a result whenever this transformer is used internally. Can also use an entire :py:class:`Citations` object. upgrade : Bool | None - Whether to include this transformer when searching for paths - between transformers. This is decided based on whether the - transformer loses important information when converting from one - type to the other. - True: Transformer does not loose infromationa and should be - included in searches. - False: Transformer does loose information but can still be used in - searches as a fall back option. - None: Transformer looses too much information and cannot be used in - searches. This is the default. + How to consider this transformer when searching for transformation + paths between formats. There are three options: + + `True` indicates that the transformer is not lossy, is usable at + any step in transformer search paths, and takes precedence over + `False`. + + `False` indicates that the transformer is lossy but can be used in + transformer search paths only if there are no other satisfactory + paths that include only `upgrade=True` transformers. + + None indicates that the transformer is lossy and can be used only + at the terminal ends of search paths. This is the default. + Returns ------- decorator From ba52e614462e5554d884c1a592c2f1b70c2dad23 Mon Sep 17 00:00:00 2001 From: Colin Wood <68213641+colinvwood@users.noreply.github.com> Date: Thu, 3 Sep 2026 11:38:08 -0700 Subject: [PATCH 7/9] lint, path validation, cohesion --- src/rachis/core/transform.py | 203 +++++++++++++++++++++-------------- 1 file changed, 123 insertions(+), 80 deletions(-) diff --git a/src/rachis/core/transform.py b/src/rachis/core/transform.py index 3663cc6c2..ab3433bfc 100644 --- a/src/rachis/core/transform.py +++ b/src/rachis/core/transform.py @@ -9,6 +9,9 @@ from enum import Enum import pathlib +from typing import TYPE_CHECKING +if TYPE_CHECKING: + from rachis.plugin.plugin import TransformerRecord from rachis import sdk from rachis.plugin import model @@ -257,7 +260,7 @@ def __init__( self, type_: type, parent: SearchNode | None, - record: 'TransformerRecord' | None = None, + record: TransformerRecord | None = None, transform_type: TransformType = TransformType.registered, wrapped: bool = False ): @@ -299,10 +302,23 @@ def __len__(self): return length def __eq__(self, other): - return self.type_ == other.type_ + ''' + Two `SearchNode`s should compare equal if they represent the same + type (vertex in the graph) and if they same class of path history. + + The second part keeps the search from short-circuiting when a node + is reached in an alternate way, which is desirable because it may be + the case that the alternate path is viable and the original one isn't. + (Due to the inclusion of `upgrade=None` in the original but not in the + alternate, for example.) + ''' + return ( + self.type_ == other.type_ + and self.classify() == other.classify() + ) def __hash__(self): - return hash(self.type_) + return hash((self.type_, self.classify())) def __repr__(self): return ( @@ -311,6 +327,60 @@ def __repr__(self): f'transform_type={self.transform_type})' ) + def classify(self) -> PathType: + ''' + Classify the path encoded in a `SearchNode` as one of `PathType`. + ''' + status = PathType.upgrade_only + node = self + while node is not None: + if node.record is not None and node.record.upgrade is False: + status = PathType.includes_false + elif node.record is not None and node.record.upgrade is None: + status = PathType.includes_none + return status + + node = node.parent + + return status + + def validate_path(self) -> bool: + ''' + Validates the transformation path encoded in the chain of parents. + Ensures that there is at most one `upgrade=None` transformation step + which, if present, occurs at one of the ends of the path. Implicit + transformations (defined elsewhere) are not considered when determining + the ends of the path. + + Accounting for `None`s is done separately here because it is + impractical to look ahead or look backwards when managing the queue in + `NodeQueue`. + + Returns + ------- + bool + Whether the path is valid. + ''' + steps = [] + node = self + while node is not None: + steps.insert(0, node) + node = node.parent + + explicit_steps = [n for n in steps if n.record is not None] + + none_count = 0 + for i, n in enumerate(explicit_steps): + if n.record.upgrade is None: + none_count += 1 + if i not in {0, len(explicit_steps) - 1}: + return False + + if none_count > 1: + return False + + return True + class NodeQueue: def __init__(self): @@ -320,28 +390,15 @@ def push(self, node: SearchNode) -> None: ''' Inserts a node and resorts the queue. - The queue is ordered according to the following algorithm. A - transformer falls into one of three categories: - - implicit transformers (wrap/unwrap transformations to and from - file formats and single file directory formats) - - explicit transformers that are marked `upgrade=True` - - explicit transformers that are marked `upgrade=False` or - `upgrade=None` - - The preference is in the order listed above, with the following - justification. Implicit transformers are the most likely to initiate - or complete the transformation path as we often begin and end with - file formats or single file directory formats. Transformers marked - `upgrade=True` are non-lossy and can be chained indefinitely. - Transformers marked `upgrade=False` or `upgrade=None` are lossy and - should be used only as a last resort. - - Within each category, paths that are shorter are preferred. + The queue is sorted primarily by `PathType` and secondarily by + path length. In both cases lower values are preferred. This ensures + that `upgrade=True`-only paths are exhausted before including + `upgrade=False` steps, and so on. ''' self.nodes.append(node) def primary(node): - return int(self.classify_node(node).value) + return int(node.classify().value) def secondary(node): return len(node) @@ -354,73 +411,56 @@ def pop(self) -> SearchNode | None: return self.nodes.pop() - def classify_node(self, node: SearchNode) -> PathType: + def insert_neighbors(self, node: SearchNode) -> None: ''' - Classify the path encoded in a `SearchNode` as one of `PathType`. - ''' - status = PathType.upgrade_only - while node is not None: - if node.record is not None and node.record.upgrade is False: - status = PathType.includes_false - elif node.record is not None and node.record.upgrade is None: - status = PathType.includes_none - return status - - node = node.parent - - return status - - -def insert_neighbors(node: SearchNode, node_queue: NodeQueue) -> None: - ''' - Find explicit and implicit neighbors to `node` and add them to - `node_queue`. + Find explicit and implicit neighbors to `node` and add them to the + queue. - Parameters - ---------- - node : SearchNode - The node the neighbors of which should be added. - node_queue : NodeQueue - The remaining nodes to search while finding a transformation path. - Neighbors are pushed into this queue. - ''' - pm = sdk.PluginManager() - - # explicit neighbors - for neighbor, transform_record in pm.transformers.get( - node.type_, {} - ).items(): - neighbor_node = SearchNode( - type_=neighbor, - parent=node, - record=transform_record, - ) - node_queue.push(neighbor_node) - - # implicit neighbors - if issubclass(node.type_, model.base.FormatBase): - # add synthetic link for Dx -> x - if issubclass(node.type_, model.SingleFileDirectoryFormatBase): - neighbor = SearchNode( - type_=node.type_.file.format, + Parameters + ---------- + node : SearchNode + The node the neighbors of which should be added. + node_queue : NodeQueue + The remaining nodes to search while finding a transformation path. + Neighbors are pushed into this queue. + ''' + pm = sdk.PluginManager() + + # explicit neighbors + for neighbor, transform_record in pm.transformers.get( + node.type_, {} + ).items(): + neighbor_node = SearchNode( + type_=neighbor, parent=node, - record=None, - transform_type=TransformType.unwrap, + record=transform_record, ) - node.wrapped = True - node_queue.push(neighbor) + self.push(neighbor_node) - # add synthetic link(s) x -> Dx - else: - for sfdf in pm._ff_to_sfdf.get(node.type_, []): + # implicit neighbors + if issubclass(node.type_, model.base.FormatBase): + # add synthetic link for Dx -> x + if issubclass(node.type_, model.SingleFileDirectoryFormatBase): neighbor = SearchNode( - type_=sfdf, + type_=node.type_.file.format, parent=node, record=None, - transform_type=TransformType.wrap, + transform_type=TransformType.unwrap, ) node.wrapped = True - node_queue.push(neighbor) + self.push(neighbor) + + # add synthetic link(s) x -> Dx + else: + for sfdf in pm._ff_to_sfdf.get(node.type_, []): + neighbor = SearchNode( + type_=sfdf, + parent=node, + record=None, + transform_type=TransformType.wrap, + ) + node.wrapped = True + self.push(neighbor) def find_transformation_path(start: type, target: type) -> SearchNode | None: @@ -455,10 +495,13 @@ def find_transformation_path(start: type, target: type) -> SearchNode | None: continue if current.type_ == target: - return current + if current.validate_path(): + return current + else: + continue visited.add(current) - insert_neighbors(current, node_queue) + node_queue.insert_neighbors(current) def compose_transformation(target: SearchNode | None, recorder = None): From f7b6f39cae4b2417ccdec9320c433688e2cbb22e Mon Sep 17 00:00:00 2001 From: Colin Wood <68213641+colinvwood@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:33:50 -0700 Subject: [PATCH 8/9] change how cycles are detected, other small refactors --- src/rachis/core/transform.py | 105 +++++++++++++++++++++-------------- 1 file changed, 62 insertions(+), 43 deletions(-) diff --git a/src/rachis/core/transform.py b/src/rachis/core/transform.py index ab3433bfc..4fe242dfd 100644 --- a/src/rachis/core/transform.py +++ b/src/rachis/core/transform.py @@ -301,25 +301,6 @@ def __len__(self): return length - def __eq__(self, other): - ''' - Two `SearchNode`s should compare equal if they represent the same - type (vertex in the graph) and if they same class of path history. - - The second part keeps the search from short-circuiting when a node - is reached in an alternate way, which is desirable because it may be - the case that the alternate path is viable and the original one isn't. - (Due to the inclusion of `upgrade=None` in the original but not in the - alternate, for example.) - ''' - return ( - self.type_ == other.type_ - and self.classify() == other.classify() - ) - - def __hash__(self): - return hash((self.type_, self.classify())) - def __repr__(self): return ( f'SearchNode(id={id(self)}, type_={repr(self.type_)}, ' @@ -361,14 +342,7 @@ def validate_path(self) -> bool: bool Whether the path is valid. ''' - steps = [] - node = self - while node is not None: - steps.insert(0, node) - node = node.parent - - explicit_steps = [n for n in steps if n.record is not None] - + explicit_steps = self.steps(explicit=True) none_count = 0 for i, n in enumerate(explicit_steps): if n.record.upgrade is None: @@ -381,6 +355,57 @@ def validate_path(self) -> bool: return True + def steps(self, explicit=False) -> list[SearchNode]: + ''' + Converts the ancestors of self into a list of `SearchNodes`s. + + Parameters + ---------- + explicit : bool + Whether to include only explicit transformation steps (i.e. those + that are registered). + + Returns + ------- + list[SearchNode] + The `SearchNode` ancestry. + ''' + steps = [] + node = self + while node is not None: + steps.insert(0, node) + node = node.parent + + if explicit: + return [n for n in steps if n.record is not None] + + return steps + + def has_ancestor(self, node: SearchNode) -> bool: + ''' + Searches for the type of `node` among the ancestors of `self`. Used to + prevent cycles during the transformation path search. + + Paremeters + ---------- + node : SearchNode + The node the type of which will be searched for among ancestors of + `self`. + + Returns + ------- + bool + Whether a matching ancestor exists. + ''' + current = self + while current is not None: + if current.type_ == node.type_: + return True + + current = current.parent + + return False + class NodeQueue: def __init__(self): @@ -430,12 +455,13 @@ def insert_neighbors(self, node: SearchNode) -> None: for neighbor, transform_record in pm.transformers.get( node.type_, {} ).items(): - neighbor_node = SearchNode( + neighbor = SearchNode( type_=neighbor, parent=node, record=transform_record, ) - self.push(neighbor_node) + if not node.has_ancestor(neighbor): + self.push(neighbor) # implicit neighbors if issubclass(node.type_, model.base.FormatBase): @@ -448,7 +474,8 @@ def insert_neighbors(self, node: SearchNode) -> None: transform_type=TransformType.unwrap, ) node.wrapped = True - self.push(neighbor) + if not node.has_ancestor(neighbor): + self.push(neighbor) # add synthetic link(s) x -> Dx else: @@ -460,7 +487,8 @@ def insert_neighbors(self, node: SearchNode) -> None: transform_type=TransformType.wrap, ) node.wrapped = True - self.push(neighbor) + if not node.has_ancestor(neighbor): + self.push(neighbor) def find_transformation_path(start: type, target: type) -> SearchNode | None: @@ -481,7 +509,6 @@ def find_transformation_path(start: type, target: type) -> SearchNode | None: A SearchNode of the target type, if reachable, otherwise None. ''' current = SearchNode(type_=start, parent=None) - visited: set[SearchNode] = set() node_queue = NodeQueue() node_queue.push(current) @@ -491,16 +518,12 @@ def find_transformation_path(start: type, target: type) -> SearchNode | None: if current is None: return None - if current in visited: + if not current.validate_path(): continue if current.type_ == target: - if current.validate_path(): - return current - else: - continue + return current - visited.add(current) node_queue.insert_neighbors(current) @@ -510,11 +533,7 @@ def compose_transformation(target: SearchNode | None, recorder = None): pm = sdk.PluginManager() - steps = [] - current = target - while current is not None: - steps.insert(0, current) - current = current.parent + steps = target.steps() if recorder is not None: for i in range(len(steps) - 1): From 4a636db776a914242273caf0bd785ccbe0487e1d Mon Sep 17 00:00:00 2001 From: Colin Wood <68213641+colinvwood@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:48:53 -0700 Subject: [PATCH 9/9] small --- src/rachis/core/transform.py | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/rachis/core/transform.py b/src/rachis/core/transform.py index 4fe242dfd..8378dd88c 100644 --- a/src/rachis/core/transform.py +++ b/src/rachis/core/transform.py @@ -463,33 +463,31 @@ def insert_neighbors(self, node: SearchNode) -> None: if not node.has_ancestor(neighbor): self.push(neighbor) - # implicit neighbors - if issubclass(node.type_, model.base.FormatBase): - # add synthetic link for Dx -> x - if issubclass(node.type_, model.SingleFileDirectoryFormatBase): + # add synthetic link for Dx -> x + if issubclass(node.type_, model.SingleFileDirectoryFormatBase): + neighbor = SearchNode( + type_=node.type_.file.format, + parent=node, + record=None, + transform_type=TransformType.unwrap, + ) + node.wrapped = True + if not node.has_ancestor(neighbor): + self.push(neighbor) + + # add synthetic link(s) x -> Dx + elif issubclass(node.type_, model.base.FormatBase): + for sfdf in pm._ff_to_sfdf.get(node.type_, []): neighbor = SearchNode( - type_=node.type_.file.format, + type_=sfdf, parent=node, record=None, - transform_type=TransformType.unwrap, + transform_type=TransformType.wrap, ) node.wrapped = True if not node.has_ancestor(neighbor): self.push(neighbor) - # add synthetic link(s) x -> Dx - else: - for sfdf in pm._ff_to_sfdf.get(node.type_, []): - neighbor = SearchNode( - type_=sfdf, - parent=node, - record=None, - transform_type=TransformType.wrap, - ) - node.wrapped = True - if not node.has_ancestor(neighbor): - self.push(neighbor) - def find_transformation_path(start: type, target: type) -> SearchNode | None: '''