diff --git a/extras/fileformats/extras/core/tests/test_converter.py b/extras/fileformats/extras/core/tests/test_converter.py index 24143492..b7962b68 100644 --- a/extras/fileformats/extras/core/tests/test_converter.py +++ b/extras/fileformats/extras/core/tests/test_converter.py @@ -6,6 +6,7 @@ from pydra.compose import python, shell from conftest import write_test_file +from fileformats.application import Tar, TarGzip, Zip from fileformats.core import ConvertibleToMarker, converter from fileformats.core.exceptions import FormatConversionError from fileformats.generic import File, FileSet @@ -101,6 +102,29 @@ def test_convert_mapped_conversion(work_dir): assert bar.raw_contents == baz.raw_contents +def archives_of(klass: type[FileSet]) -> tuple[type[FileSet], ...]: + """The archive types that any type can be extracted from by the generic converters + from archives, e.g. `Zip[Compressed]` -> `Compressed`""" + return (TarGzip[klass], Tar[klass], Zip[klass]) + + +def split_convertible_from( + klass: type[FileSet], **kwargs: ty.Any +) -> tuple[tuple[type[FileSet], ...], tuple[type[FileSet], ...]]: + """Splits the types that `klass` is convertible from into the generic ones (i.e. + the source types of generic converters with their wildcards resolved to `klass`, + e.g. `Zip[klass]`) and the rest. Which generic converters are registered depends + on the other tests that have been run (e.g. the classifier tests), so the generic + ones can only be checked for the archive types""" + hint = klass.convertible_from(**kwargs) + assert ConvertibleToMarker.from_hint(hint) == ConvertibleToMarker(klass) + members = ty.get_args(ty.get_args(hint)[0]) or (ty.get_args(hint)[0],) + generic = tuple( + t for t in members if klass in getattr(t, "classifiers", ()) and t is not klass + ) + return tuple(t for t in members if t not in generic), generic + + @pytest.mark.parametrize( ["klass", "convertible_from"], [ @@ -115,7 +139,6 @@ def test_convert_mapped_conversion(work_dir): ], ) def test_convertible_from(klass: type[FileSet], convertible_from: type[FileSet]): - assert ( - klass.convertible_from() - == ty.Annotated[convertible_from, ConvertibleToMarker(klass)] - ) + direct, generic = split_convertible_from(klass) + assert set(direct) == set(ty.get_args(convertible_from) or (convertible_from,)) + assert set(archives_of(klass)).issubset(generic) diff --git a/extras/fileformats/extras/core/tests/test_convertible.py b/extras/fileformats/extras/core/tests/test_convertible.py index 5f954cc9..7db87771 100644 --- a/extras/fileformats/extras/core/tests/test_convertible.py +++ b/extras/fileformats/extras/core/tests/test_convertible.py @@ -2,9 +2,7 @@ import typing as ty import pytest - -# Registers the converters between the testing formats -import test_converter # noqa: F401 +from test_converter import archives_of, split_convertible_from from fileformats.core import ( ConvertibleTo, @@ -21,19 +19,24 @@ def test_convertible_to_hint(): hint = ConvertibleTo[Bar] assert ty.get_origin(hint) is ty.Annotated - assert ty.get_args(hint)[0] == Bar | Baz | Foo + direct, generic = split_convertible_from(Bar) + assert set(direct) == {Bar, Baz, Foo} + assert set(archives_of(Bar)).issubset(generic) marker = ConvertibleToMarker.from_hint(hint) assert marker == ConvertibleToMarker(Bar) assert marker.format is Bar assert hint == Bar.convertible_from() assert marker.hint() == hint - assert marker.resolve() == Bar | Baz | Foo + assert marker.resolve() == ty.get_args(hint)[0] assert LoadedMarker.from_hint(hint) is None def test_convertible_no_converters(): hint = ConvertibleTo[Baz] - assert ty.get_args(hint)[0] is Baz + # Only generic converters (e.g. from archives) can convert to Baz + direct, generic = split_convertible_from(Baz) + assert direct == (Baz,) + assert set(archives_of(Baz)).issubset(generic) assert ConvertibleToMarker.from_hint(hint).format is Baz @@ -57,17 +60,21 @@ def test_convertible_requires_format(): @pytest.mark.parametrize( - ["hint", "mime"], + ["make_hint", "mime"], [ - [ConvertibleTo[Bar], "testing/bar+convertible-to"], - [ConvertibleTo[Foo], "testing/foo+convertible-to"], - [ConvertibleTo[Baz], "testing/baz+convertible-to"], - [ConvertibleTo[Bar] | None, "testing/bar+convertible-to?"], - [list[ConvertibleTo[Bar]], "testing/bar+convertible-to+list-of"], - [ConvertibleTo[Bar] | Qux, "testing/bar+convertible-to|testing/qux"], + [lambda: ConvertibleTo[Bar], "testing/bar+convertible-to"], + [lambda: ConvertibleTo[Foo], "testing/foo+convertible-to"], + [lambda: ConvertibleTo[Baz], "testing/baz+convertible-to"], + [lambda: ConvertibleTo[Bar] | None, "testing/bar+convertible-to?"], + [lambda: list[ConvertibleTo[Bar]], "testing/bar+convertible-to+list-of"], + [lambda: ConvertibleTo[Bar] | Qux, "testing/bar+convertible-to|testing/qux"], ], ) -def test_convertible_mime_roundtrip(hint: ty.Any, mime: str): +def test_convertible_mime_roundtrip(make_hint: ty.Callable[[], ty.Any], mime: str): + # The hints are created in the test, rather than when the parameters are collected, + # as the types they evaluate to depend on the converters that have been registered + # by then (e.g. generic converters registered by other test modules) + hint = make_hint() assert to_mime(hint, official=False) == mime assert from_mime(mime) == hint @@ -90,8 +97,6 @@ def test_convertible_sorted_by_specificity(): assert len(args) > 1 for i, tp in enumerate(args): assert not any(issubclass(later, tp) for later in args[i + 1 :]) - assert ty.get_args(ty.get_args(Png.convertible_from())[0]) == (Png, RasterImage) - by_name = ty.get_args( - ty.get_args(Bar.convertible_from(union_sort_key=lambda t: t.__name__))[0] - ) + assert split_convertible_from(Png)[0] == (Png, RasterImage) + by_name = split_convertible_from(Bar, union_sort_key=lambda t: t.__name__)[0] assert by_name == (Bar, Baz, Foo) diff --git a/extras/fileformats/extras/core/tests/test_convertible_from_wildcards.py b/extras/fileformats/extras/core/tests/test_convertible_from_wildcards.py new file mode 100644 index 00000000..213e7cbe --- /dev/null +++ b/extras/fileformats/extras/core/tests/test_convertible_from_wildcards.py @@ -0,0 +1,46 @@ +"""The source types of generic converters (i.e. with wildcard classifiers) are +included in `convertible_from()` with their wildcards resolved from the target type""" + +import typing as ty + +from fileformats.application import Tar, TarGzip, Zip +from fileformats.core import FileSet +from fileformats.generic import Directory, DirectoryOf, SetOf +from fileformats.text import TextFile + + +def members(datatype: type[FileSet]) -> tuple[type, ...]: + union = ty.get_args(datatype.convertible_from())[0] + return ty.get_args(union) or (union,) + + +def test_convertible_from_generic_container_conversion() -> None: + """`SetOf[T]` -> `DirectoryOf[T]` is registered on the unclassified `DirectoryOf`, + so `T` is resolved from the classifier of the target""" + assert SetOf[TextFile] in members(DirectoryOf[TextFile]) + assert DirectoryOf[TextFile].get_converter(SetOf[TextFile]) is not None + + +def test_convertible_from_archives() -> None: + """`Zip[Compressed]` -> `Compressed` etc. can extract any type from an archive""" + for archive in (Zip, Tar, TarGzip): + assert archive[TextFile] in members(TextFile) + assert TextFile.get_converter(archive[TextFile]) is not None + + +def test_convertible_from_wildcard_source() -> None: + """`Compressed` -> `Zip[Compressed]` converts a type into an archive of it, so the + archived type is resolved from the classifier of the target""" + assert TextFile in members(Zip[TextFile]) + + +def test_convertible_from_has_no_wildcard_types() -> None: + """Unresolved source types of generic converters (e.g. `SetOf[T]`) can't be + instantiated, so aren't included for types they can't be resolved from (i.e. + unclassified ones)""" + for datatype in (FileSet, Directory, TextFile): + assert not [ + t + for t in members(datatype) + if hasattr(t, "wildcard_classifiers") and t.wildcard_classifiers() + ] diff --git a/fileformats/core/fileset.py b/fileformats/core/fileset.py index 945ce447..5d87daf4 100644 --- a/fileformats/core/fileset.py +++ b/fileformats/core/fileset.py @@ -1011,7 +1011,16 @@ def convertible_from( for subcls in subclasses: if issubclass(subcls, cls): subcls._import_extras_module() - datatypes.extend(subcls.get_converters_dict().keys()) + # Skip the source types of generic converters (e.g. `SetOf[T]` for + # `SetOf[T]` -> `DirectoryOf[T]`), which can't be used as concrete + # datatypes, and are added below with their wildcards resolved from the + # classifiers of this type where possible + datatypes.extend( + t + for t in subcls.get_converters_dict().keys() + if not _has_wildcard_classifiers(t) + ) + datatypes.extend(cls._resolved_wildcard_sources()) marker = ConvertibleToMarker(cls) if len(datatypes) == 1: return ty.Annotated[cls, marker] @@ -1036,6 +1045,79 @@ def add_concrete(datatype: ty.Type[DataType]) -> None: ) return ty.Annotated[union, marker] + @classmethod + def _resolved_wildcard_sources(cls) -> ty.List[ty.Type[DataType]]: + """The source types of the generic converters that can convert to this type, + with their wildcard classifiers resolved from this type, e.g. `Zip[TextFile]` + for `TextFile` (from `Zip[Compressed]` -> `Compressed`) and `SetOf[TextFile]` + for `DirectoryOf[TextFile]` (from `SetOf[T]` -> `DirectoryOf[T]`). Only those + that a converter can actually be found from with `get_converter` are returned. + """ + from .converter_helpers import SubtypeVar + + if _has_wildcard_classifiers(cls): + return [] # nothing to resolve the wildcards from + # The extras modules of generically classifiable types (e.g. archives) register + # generic converters to and from them for any type (e.g. `Zip[X]` -> `X`), so + # import them, so the result doesn't depend on which happen to be imported + for generic_type in set(DataType.generically_classifiable_by_name.values()): + generic_type._import_extras_module() # type: ignore[attr-defined] + + # The source types of generic converters, and the types their wildcards resolve + # to for this type + candidates: ty.List[ + ty.Tuple[ty.Type[DataType], ty.Dict[ty.Type[DataType], ty.Type[DataType]]] + ] = [] + # Converters from generic types to the types they are classified by, e.g. + # `Zip[Compressed]` -> `Compressed` + for extracted_from in SubtypeVar.converters: + (wildcard,) = extracted_from.wildcard_classifiers() # type: ignore[attr-defined] + if issubclass(cls, wildcard.bound): # type: ignore[attr-defined] + candidates.append((extracted_from, {wildcard: cls})) + # Converters between generic types, or to a generic type, registered on the + # unclassified type, e.g. `SetOf[T]` -> `DirectoryOf[T]` and + # `Compressed` -> `Zip[Compressed]`, where the wildcard is resolved from the + # classifier of this type in place of the target's wildcard + if getattr(cls, "is_classified", False): + converters_dict = FileSet.get_converters_dict( + cls.unclassified # type: ignore[attr-defined] + ) + for source_template, generic_converter in converters_dict.items(): + if not generic_converter.classifiers: + continue # not a generic converter + wildcards = cls.wildcard_classifiers( # type: ignore[attr-defined] + generic_converter.classifiers + ) + non_wildcards = cls.non_wildcard_classifiers( # type: ignore[attr-defined] + generic_converter.classifiers + ) + actual = set(cls.classifiers).difference( # type: ignore[attr-defined] + non_wildcards + ) + if ( + len(wildcards) != 1 + or len(actual) != 1 + or not non_wildcards.issubset(cls.classifiers) # type: ignore[attr-defined] + ): + continue + candidates.append( + (source_template, {next(iter(wildcards)): next(iter(actual))}) + ) + resolved = [] + for unresolved, mapping in candidates: + try: + # Resolving the wildcards can produce an invalid type (e.g. one with + # classifiers that clash), in which case there is nothing to add + candidate = _resolve_wildcards(unresolved, mapping) + if candidate is cls: + continue + found = cls.get_converter(candidate) + except (FormatConversionError, FormatDefinitionError): + continue + if found is not None: # i.e. not just a subclass of this type + resolved.append(candidate) + return resolved + @classmethod def get_converter_defs(cls, source_format: type) -> ty.List["Converter"]: """Search the registered converters to find any matches and return list of @@ -2415,3 +2497,31 @@ def _root_dir(fileset: FileSet) -> Path: """The directory containing all of the fileset's paths (the parent of a directory fileset rather than the directory itself)""" return Path(os.path.commonpath([p.parent for p in fileset.fspaths])) + + +def _has_wildcard_classifiers(datatype: ty.Type[DataType]) -> bool: + """Whether the type is classified by wildcards (e.g. `SetOf[T]`), i.e. a template + for the source or target types of generic converters""" + try: + return bool(datatype.wildcard_classifiers()) # type: ignore[attr-defined] + except (AttributeError, TypeError): + return False + + +def _resolve_wildcards( + template: ty.Type[DataType], + mapping: ty.Mapping[ty.Type[DataType], ty.Type[DataType]], +) -> ty.Type[DataType]: + """Substitute the wildcard classifiers of a generic converter's source type with the + types they resolve to, e.g. `SetOf[T]` with {T: TextFile} -> `SetOf[TextFile]`, or + the type itself if the source type is the wildcard, e.g. `Compressed` -> `TextFile` + """ + from .converter_helpers import SubtypeVar + + if issubclass(template, SubtypeVar): + return mapping[template] + classifiers = tuple( + mapping.get(c, c) for c in template.classifiers # type: ignore[attr-defined] + ) + unclassified = template.unclassified # type: ignore[attr-defined] + return unclassified[classifiers if len(classifiers) > 1 else classifiers[0]] # type: ignore[no-any-return] diff --git a/fileformats/core/mixin.py b/fileformats/core/mixin.py index 2a400d4b..2aaf3044 100644 --- a/fileformats/core/mixin.py +++ b/fileformats/core/mixin.py @@ -685,11 +685,17 @@ def register_converter( f"({list(source_wildcard_classifiers)}), and target " f"{cls} ({cls.wildcard_classifiers()})" ) + # Check the converters registered on the unclassified type, which is + # where generic converters are stored (see below), rather than + # `cls.converters`, which may be inherited from a base class (e.g. + # `FsObject`) and hold converters from types that aren't classified + converters_dict = cls.unclassified.get_converters_dict() # type: ignore[attr-defined] prev_registered = [ f - for f in cls.converters # type: ignore[attr-defined] + for f in converters_dict if ( - issubclass(source_format.unclassified, f.unclassified) # type: ignore[attr-defined] + getattr(f, "is_classified", False) + and issubclass(source_format.unclassified, f.unclassified) # type: ignore[attr-defined] and f.non_wildcard_classifiers() == source_format.non_wildcard_classifiers() # type: ignore[attr-defined] ) @@ -697,7 +703,7 @@ def register_converter( assert len(prev_registered) <= 1 prev = prev_registered[0] if prev_registered else None if prev: - prev_converter = cls.converters[prev] # type: ignore[attr-defined] + prev_converter = converters_dict[prev] # task, task_kwargs = converter_spec # prev_task, prev_kwargs, prev_classifiers = prev_tuple if converter == prev_converter: