From ffd096b53be6589a76666bf7e897931145c5e175 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Wed, 18 Feb 2026 22:13:08 -0800 Subject: [PATCH 1/6] [Draft] Literal Include Quick draft w/ Cursor on include support. --- src/pals/functions.py | 94 ++++++++++++++++++++++++-- src/pals/kinds/Lattice.py | 7 +- tests/test_include.py | 139 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 233 insertions(+), 7 deletions(-) create mode 100644 tests/test_include.py diff --git a/src/pals/functions.py b/src/pals/functions.py index 4186f6f..060b2ce 100644 --- a/src/pals/functions.py +++ b/src/pals/functions.py @@ -3,7 +3,7 @@ import os -def inspect_file_extensions(filename: str): +def inspect_file_extensions(filename: str, check_extension: bool = True): """Attempt to strip two levels of file extensions to determine the schema. filename examples: fodo.pals.yaml, fodo.pals.json, ... @@ -11,7 +11,7 @@ def inspect_file_extensions(filename: str): file_noext, extension = os.path.splitext(filename) file_noext_noext, extension_inner = os.path.splitext(file_noext) - if extension_inner != ".pals": + if check_extension and extension_inner != ".pals": raise RuntimeError( f"inspect_file_extensions: No support for file {filename} with extension {extension}. " f"PALS files must end in .pals.json or .pals.yaml or similar." @@ -25,11 +25,94 @@ def inspect_file_extensions(filename: str): } -def load_file_to_dict(filename: str) -> dict: +def process_includes(data, base_dir: str): + """Recursively process 'include' directives in the data structure.""" + if isinstance(data, dict): + # Handle 'include' key in dictionary + if "include" in data: + include_file = data["include"] + # Check if include_file is a string (filename) + if isinstance(include_file, str): + filepath = os.path.join(base_dir, include_file) + # Load included file without strict extension check + included_data = load_file_to_dict(filepath, check_extension=False) + + # Remove 'include' key + local_data = data.copy() + del local_data["include"] + + # Recursively process local data + local_data = { + k: process_includes(v, base_dir) for k, v in local_data.items() + } + + # Merge logic + # If included data is a list of single-key dicts (PALS special case), try to merge as dict + if isinstance(included_data, list): + try: + merged_included = {} + all_dicts = True + for item in included_data: + if isinstance(item, dict) and len(item) == 1: + merged_included.update(item) + else: + all_dicts = False + break + if all_dicts: + included_data = merged_included + except Exception: + pass + + if isinstance(included_data, dict): + # Merge included data with local data (local overrides included?) + # Spec: "Included file data will be included verbatim at the current level of nesting." + # Usually specific (local) overrides generic (included). + # So we take included, update with local. + result = included_data.copy() + result.update(local_data) + return result + else: + # If included data is not a dict, we can't merge it into a dict. + # Unless the dict was JUST the include? + if not local_data: + return included_data + # Fallback: return local data (ignore include) or error? + # For now, let's return local_data but maybe warn? + # Or maybe return included_data if local_data is empty? + return local_data + + # Recurse on values if no include or after processing + return {k: process_includes(v, base_dir) for k, v in data.items()} + + elif isinstance(data, list): + new_list = [] + for item in data: + # Check if item is a dict with ONLY 'include' key + if isinstance(item, dict) and "include" in item and len(item) == 1: + include_file = item["include"] + if isinstance(include_file, str): + filepath = os.path.join(base_dir, include_file) + included_data = load_file_to_dict(filepath, check_extension=False) + + if isinstance(included_data, list): + new_list.extend(included_data) + else: + new_list.append(included_data) + else: + new_list.append(process_includes(item, base_dir)) + else: + new_list.append(process_includes(item, base_dir)) + return new_list + + else: + return data + + +def load_file_to_dict(filename: str, check_extension: bool = True) -> dict: # Attempt to strip two levels of file extensions to determine the schema. # Examples: fodo.pals.yaml, fodo.pals.json, ... file_noext, extension, file_noext_noext, extension_inner = inspect_file_extensions( - filename + filename, check_extension=check_extension ).values() # examples: fodo.pals.yaml, fodo.pals.json @@ -51,6 +134,9 @@ def load_file_to_dict(filename: str) -> dict: f"load_file_to_dict: No support for PALS file {filename} with extension {extension} yet." ) + # Process includes + pals_data = process_includes(pals_data, base_dir=os.path.dirname(filename)) + return pals_data diff --git a/src/pals/kinds/Lattice.py b/src/pals/kinds/Lattice.py index c8253ad..c238e5b 100644 --- a/src/pals/kinds/Lattice.py +++ b/src/pals/kinds/Lattice.py @@ -1,7 +1,8 @@ -from pydantic import model_validator, Field -from typing import Annotated, List, Literal, Union +from pydantic import model_validator +from typing import List, Literal, Union from .BeamLine import BeamLine +from .PlaceholderName import PlaceholderName from .mixin import BaseElement from ..functions import load_file_to_dict, store_dict_to_file @@ -11,7 +12,7 @@ class Lattice(BaseElement): kind: Literal["Lattice"] = "Lattice" - branches: List[Annotated[Union[BeamLine], Field(discriminator="kind")]] + branches: List[Union[BeamLine, PlaceholderName]] @model_validator(mode="before") @classmethod diff --git a/tests/test_include.py b/tests/test_include.py new file mode 100644 index 0000000..1163a04 --- /dev/null +++ b/tests/test_include.py @@ -0,0 +1,139 @@ +import pals + + +def test_include(tmp_path): + main_file = tmp_path / "main.pals.yaml" + root_included_file = tmp_path / "included.pals.yaml" + facility_included_file = tmp_path / "facility.pals.yaml" + facility_nested_file = tmp_path / "facility_nested.pals.yaml" + + main_content = f""" + PALS: + include: "{root_included_file.name}" + facility: + - drift1: + kind: Drift + length: 0.25 + + - include: "{facility_included_file.name}" + + - fodo_cell: + kind: BeamLine + line: + - drift1 + - quad1 + - drift2 + - quad2 + - drift1 + + - fodo_lattice: + kind: Lattice + branches: + - fodo_cell + + - use: fodo_lattice + """ + + root_included_content = """ + author: "Some One " + version: 1.0 + """ + + facility_included_content = f""" + - quad1: + kind: Quadrupole + MagneticMultipoleP: + Bn1: 1.0 + length: 1.0 + + - drift2: + kind: Drift + length: 0.5 + + - include: "{facility_nested_file.name}" + """ + + facility_nested_content = """ + - quad2: + kind: Quadrupole + MagneticMultipoleP: + Bn1: -1.0 + length: 1.0 + """ + + main_file.write_text(main_content) + root_included_file.write_text(root_included_content) + facility_included_file.write_text(facility_included_content) + facility_nested_file.write_text(facility_nested_content) + + data = pals.Lattice.from_file(main_file) + + assert data["PALS"]["version"] == 1.0 + assert data["PALS"]["other"] == "value" + assert data["PALS"]["author"] == "Some One " + assert "include" not in data["PALS"] + + +def test_nested_include(tmp_path): + root_file = tmp_path / "root.pals.yaml" + middle_file = tmp_path / "middle.pals.yaml" + leaf_file = tmp_path / "leaf.pals.yaml" + + root_content = f""" + root: + include: "{middle_file.name}" + """ + + middle_content = f""" + middle: val + include: "{leaf_file.name}" + """ + + leaf_content = """ + leaf: val + """ + + root_file.write_text(root_content) + middle_file.write_text(middle_content) + leaf_file.write_text(leaf_content) + + data = pals.functions.load_file_to_dict(str(root_file)) + + assert data["root"]["middle"] == "val" + assert data["root"]["leaf"] == "val" + assert "include" not in data["root"] + + +def test_include_list_into_dict_conversion(tmp_path): + # This tests the spec example where a list of properties is included into a dict (element) + main_file = tmp_path / "element.pals.yaml" + params_file = tmp_path / "params.pals.yaml" + + main_content = f""" + element: + kind: Quadrupole + include: "{params_file.name}" + """ + + # params file content is a list of single-key dicts + params_content = """ + - MagneticMultipoleP: + - Kn3L: 0.3 + - ApertureP: + x_limits: [-0.1, 0.1] + """ + + main_file.write_text(main_content) + params_file.write_text(params_content) + + data = pals.functions.load_file_to_dict(str(main_file)) + + elem = data["element"] + assert elem["kind"] == "Quadrupole" + + # Check if keys are correctly merged from the list + assert "MagneticMultipoleP" in elem + assert elem["MagneticMultipoleP"] == [{"Kn3L": 0.3}] + + assert "ApertureP" in elem + assert elem["ApertureP"] == {"x_limits": [-0.1, 0.1]} From a1d08e67594cef36f9d8be39b9cc6f3123464b4e Mon Sep 17 00:00:00 2001 From: Edoardo Zoni Date: Mon, 8 Jun 2026 13:19:18 -0700 Subject: [PATCH 2/6] Remove template assertion from test --- tests/test_include.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_include.py b/tests/test_include.py index 1163a04..c519924 100644 --- a/tests/test_include.py +++ b/tests/test_include.py @@ -69,7 +69,6 @@ def test_include(tmp_path): data = pals.Lattice.from_file(main_file) assert data["PALS"]["version"] == 1.0 - assert data["PALS"]["other"] == "value" assert data["PALS"]["author"] == "Some One " assert "include" not in data["PALS"] From 1b437b4534d61082815119187f3273b2135b071c Mon Sep 17 00:00:00 2001 From: Edoardo Zoni Date: Mon, 22 Jun 2026 15:02:25 -0700 Subject: [PATCH 3/6] Support loading selected lattices from full PALS root files --- src/pals/PALS.py | 7 +++++-- src/pals/kinds/Lattice.py | 22 ++++++++++++++++++++++ tests/test_include.py | 13 +++++++++---- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/pals/PALS.py b/src/pals/PALS.py index c948b73..f93888e 100644 --- a/src/pals/PALS.py +++ b/src/pals/PALS.py @@ -1,4 +1,4 @@ -from pydantic import BaseModel +from pydantic import BaseModel, ConfigDict from pydantic import model_validator from typing import Self @@ -14,7 +14,10 @@ class PALSroot(BaseModel): """Represent the roo PALS structure""" - version: str | None = None + # Preserve root-level standard metadata that is not modeled explicitly yet. + model_config = ConfigDict(extra="allow") + + version: str | int | float | None = None facility: Facility diff --git a/src/pals/kinds/Lattice.py b/src/pals/kinds/Lattice.py index 5610e8e..6de5eb8 100644 --- a/src/pals/kinds/Lattice.py +++ b/src/pals/kinds/Lattice.py @@ -32,6 +32,28 @@ def model_dump(self, *args, **kwargs): def from_file(filename: str) -> Self: """Load a Lattice from a text file""" pals_dict = load_file_to_dict(filename) + + if isinstance(pals_dict, dict) and "PALS" in pals_dict: + # Full PALS documents select their active lattice with a facility-level use. + from pals.PALS import PALSroot + from pals.kinds.PlaceholderName import PlaceholderName + + pals_root = PALSroot(**pals_dict) + use_name = None + for item in pals_root.facility: + if isinstance(item, PlaceholderName): + use_name = item.name + + if use_name is None: + raise ValueError("PALS root document does not specify a lattice to use") + + # Return the selected lattice while preserving the existing direct-lattice path. + for item in pals_root.facility: + if isinstance(item, Lattice) and item.name == use_name: + return item + + raise ValueError(f"PALS root document does not define lattice {use_name!r}") + return Lattice(**pals_dict) def to_file(self, filename: str): diff --git a/tests/test_include.py b/tests/test_include.py index c519924..19d46f2 100644 --- a/tests/test_include.py +++ b/tests/test_include.py @@ -66,11 +66,16 @@ def test_include(tmp_path): facility_included_file.write_text(facility_included_content) facility_nested_file.write_text(facility_nested_content) - data = pals.Lattice.from_file(main_file) + lattice = pals.Lattice.from_file(main_file) - assert data["PALS"]["version"] == 1.0 - assert data["PALS"]["author"] == "Some One " - assert "include" not in data["PALS"] + assert lattice.name == "fodo_lattice" + assert lattice.branches[0] == "fodo_cell" + + data = pals.PALSroot.from_file(main_file) + + assert data.version == 1.0 + assert data.author == "Some One " + assert not hasattr(data, "include") def test_nested_include(tmp_path): From f6851f1c3263f8912a83d68af55567cb292852c7 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Thu, 30 Jul 2026 16:04:16 -0700 Subject: [PATCH 4/6] Review pass over the include draft Tighten the include resolution to the standard's wording (included file data is spliced verbatim at the current level of nesting): - Sub-level included files keep an extension check instead of none: the standard's .subpals inner suffix is accepted alongside .pals, anything else still errors. - Structural mismatches (a sequence included at a mapping level, a scalar at a sequence level) raise a clear error instead of being silently dropped or reshaped; the speculative list-of-one-key-dicts merge is removed. - Include cycles are detected via the chain of including files and reported, instead of recursing until the interpreter gives up. Diamond-shaped includes remain allowed. - Lattice.from_file now follows the standard's use statement: the last Lattice defined is instantiated by default and a `use` entry overrides it; documents without a facility no longer crash it. The tests use .subpals.yaml fragment names, cover the standard's element-parameter include example, relative resolution from the including file's own directory, the error paths, and the use-statement selection rules. machine/machine.pals.yaml and unit_tests/loading/include/sub/layout.pals.yaml leave the standard-examples known-failures list. Co-Authored-By: Claude Fable 5 --- src/pals/functions.py | 165 ++++++----- src/pals/kinds/Lattice.py | 38 ++- tests/standard_examples_known_failures.txt | 4 - tests/test_include.py | 323 ++++++++++++++------- 4 files changed, 325 insertions(+), 205 deletions(-) diff --git a/src/pals/functions.py b/src/pals/functions.py index 04ab80e..daa9238 100644 --- a/src/pals/functions.py +++ b/src/pals/functions.py @@ -3,18 +3,24 @@ import os -def inspect_file_extensions(filename: str, check_extension: bool = True): +def inspect_file_extensions(filename: str, sub_level: bool = False): """Attempt to strip two levels of file extensions to determine the schema. filename examples: fodo.pals.yaml, fodo.pals.json, ... + + Sub-level files, spliced into another file by its `include` entries, use + the inner extension .subpals per the standard's File Formats section + (e.g. elements.subpals.yaml); .pals is accepted for them as well. """ file_noext, extension = os.path.splitext(filename) file_noext_noext, extension_inner = os.path.splitext(file_noext) - if check_extension and extension_inner != ".pals": + allowed_inner = (".pals", ".subpals") if sub_level else (".pals",) + if extension_inner not in allowed_inner: + expected = " or ".join(f"{inner}.yaml" for inner in allowed_inner) raise RuntimeError( f"inspect_file_extensions: No support for file {filename} with extension {extension}. " - f"PALS files must end in .pals.json or .pals.yaml or similar." + f"PALS files must end in {expected} or similar." ) return { @@ -25,94 +31,97 @@ def inspect_file_extensions(filename: str, check_extension: bool = True): } -def process_includes(data, base_dir: str): - """Recursively process 'include' directives in the data structure.""" +def _load_included_file(include_file, base_dir: str, include_chain: tuple): + """Load the target of one `include` entry, relative to the including file.""" + if not isinstance(include_file, str): + raise TypeError( + f"process_includes: an 'include' value must be a file name string, " + f"but we got {include_file!r}" + ) + filepath = os.path.join(base_dir, include_file) + return load_file_to_dict(filepath, sub_level=True, _include_chain=include_chain) + + +def process_includes(data, base_dir: str, include_chain: tuple = ()): + """Recursively resolve `include` entries in the data structure. + + Per the standard, included file data is included verbatim at the current + level of nesting: an `include` key in a mapping splices the included + mapping's entries into it (entries local to the mapping win), and a list + item holding only an `include` key splices the included sequence into the + list. Include file names are resolved relative to the including file. + + Args: + data: The parsed data structure to resolve + base_dir: Directory of the file the data came from + include_chain: Files on the include path so far, for cycle detection + + Returns: + The data structure with all includes resolved + """ if isinstance(data, dict): - # Handle 'include' key in dictionary if "include" in data: - include_file = data["include"] - # Check if include_file is a string (filename) - if isinstance(include_file, str): - filepath = os.path.join(base_dir, include_file) - # Load included file without strict extension check - included_data = load_file_to_dict(filepath, check_extension=False) - - # Remove 'include' key - local_data = data.copy() - del local_data["include"] - - # Recursively process local data - local_data = { - k: process_includes(v, base_dir) for k, v in local_data.items() - } - - # Merge logic - # If included data is a list of single-key dicts (PALS special case), try to merge as dict - if isinstance(included_data, list): - try: - merged_included = {} - all_dicts = True - for item in included_data: - if isinstance(item, dict) and len(item) == 1: - merged_included.update(item) - else: - all_dicts = False - break - if all_dicts: - included_data = merged_included - except Exception: - pass - - if isinstance(included_data, dict): - # Merge included data with local data (local overrides included?) - # Spec: "Included file data will be included verbatim at the current level of nesting." - # Usually specific (local) overrides generic (included). - # So we take included, update with local. - result = included_data.copy() - result.update(local_data) - return result - else: - # If included data is not a dict, we can't merge it into a dict. - # Unless the dict was JUST the include? - if not local_data: - return included_data - # Fallback: return local data (ignore include) or error? - # For now, let's return local_data but maybe warn? - # Or maybe return included_data if local_data is empty? - return local_data - - # Recurse on values if no include or after processing - return {k: process_includes(v, base_dir) for k, v in data.items()} + included_data = _load_included_file( + data["include"], base_dir, include_chain + ) + if not isinstance(included_data, dict): + raise TypeError( + f"process_includes: file {data['include']!r} is included at a " + f"mapping level and must hold a mapping, " + f"but we got {type(included_data).__name__}" + ) + local_data = { + key: process_includes(value, base_dir, include_chain) + for key, value in data.items() + if key != "include" + } + # Entries local to the including mapping win over included ones. + return {**included_data, **local_data} + + return { + key: process_includes(value, base_dir, include_chain) + for key, value in data.items() + } elif isinstance(data, list): new_list = [] for item in data: - # Check if item is a dict with ONLY 'include' key - if isinstance(item, dict) and "include" in item and len(item) == 1: - include_file = item["include"] - if isinstance(include_file, str): - filepath = os.path.join(base_dir, include_file) - included_data = load_file_to_dict(filepath, check_extension=False) - - if isinstance(included_data, list): - new_list.extend(included_data) - else: - new_list.append(included_data) + # A list item holding only an include splices in the included file + if isinstance(item, dict) and set(item) == {"include"}: + included_data = _load_included_file( + item["include"], base_dir, include_chain + ) + if isinstance(included_data, list): + new_list.extend(included_data) + elif isinstance(included_data, dict): + new_list.append(included_data) else: - new_list.append(process_includes(item, base_dir)) + raise TypeError( + f"process_includes: file {item['include']!r} is included at a " + f"sequence level and must hold a sequence or mapping, " + f"but we got {type(included_data).__name__}" + ) else: - new_list.append(process_includes(item, base_dir)) + new_list.append(process_includes(item, base_dir, include_chain)) return new_list else: return data -def load_file_to_dict(filename: str, check_extension: bool = True) -> dict: +def load_file_to_dict( + filename: str, sub_level: bool = False, _include_chain: tuple = () +) -> dict: + # Guard against include cycles: a file including itself through any chain. + filepath = os.path.abspath(filename) + if filepath in _include_chain: + chain = " -> ".join(_include_chain + (filepath,)) + raise RuntimeError(f"load_file_to_dict: circular include: {chain}") + # Attempt to strip two levels of file extensions to determine the schema. # Examples: fodo.pals.yaml, fodo.pals.json, ... file_noext, extension, file_noext_noext, extension_inner = inspect_file_extensions( - filename, check_extension=check_extension + filename, sub_level=sub_level ).values() # examples: fodo.pals.yaml, fodo.pals.json @@ -134,8 +143,12 @@ def load_file_to_dict(filename: str, check_extension: bool = True) -> dict: f"load_file_to_dict: No support for PALS file {filename} with extension {extension} yet." ) - # Process includes - pals_data = process_includes(pals_data, base_dir=os.path.dirname(filename)) + # Resolve include entries, tracking this file for cycle detection + pals_data = process_includes( + pals_data, + base_dir=os.path.dirname(filename), + include_chain=_include_chain + (filepath,), + ) return pals_data diff --git a/src/pals/kinds/Lattice.py b/src/pals/kinds/Lattice.py index 6de5eb8..71845c5 100644 --- a/src/pals/kinds/Lattice.py +++ b/src/pals/kinds/Lattice.py @@ -30,29 +30,35 @@ def model_dump(self, *args, **kwargs): @staticmethod def from_file(filename: str) -> Self: - """Load a Lattice from a text file""" + """Load a Lattice from a text file. + + The file can hold either a single Lattice or a full PALS document. + Per the standard's use statement, the lattice instantiated from a full + document is the last one defined, unless a `use` entry selects another. + """ pals_dict = load_file_to_dict(filename) if isinstance(pals_dict, dict) and "PALS" in pals_dict: - # Full PALS documents select their active lattice with a facility-level use. from pals.PALS import PALSroot from pals.kinds.PlaceholderName import PlaceholderName pals_root = PALSroot(**pals_dict) - use_name = None - for item in pals_root.facility: - if isinstance(item, PlaceholderName): - use_name = item.name - - if use_name is None: - raise ValueError("PALS root document does not specify a lattice to use") - - # Return the selected lattice while preserving the existing direct-lattice path. - for item in pals_root.facility: - if isinstance(item, Lattice) and item.name == use_name: - return item - - raise ValueError(f"PALS root document does not define lattice {use_name!r}") + facility = pals_root.facility or [] + lattices = [item for item in facility if isinstance(item, Lattice)] + if not lattices: + raise ValueError( + f"PALS root document {filename!r} does not define a Lattice" + ) + by_name = {lattice.name: lattice for lattice in lattices} + + # `use` entries are stored as name references; the last one naming + # a defined Lattice wins. References to non-Lattice elements + # (e.g. facility-level commands) do not select anything. + for item in reversed(facility): + if isinstance(item, PlaceholderName) and item.name in by_name: + return by_name[item.name] + + return lattices[-1] return Lattice(**pals_dict) diff --git a/tests/standard_examples_known_failures.txt b/tests/standard_examples_known_failures.txt index 73bf3dd..ddd81f7 100644 --- a/tests/standard_examples_known_failures.txt +++ b/tests/standard_examples_known_failures.txt @@ -4,10 +4,6 @@ # to load as an error, so this list shrinks as support lands. Blank lines and # `#` comments are ignored. -# `include` entries inside a facility are not resolved. -machine/machine.pals.yaml -unit_tests/loading/include/sub/layout.pals.yaml - # The sequence form of `variables` and the compact `sets` form are not # modeled. unit_tests/expressions/inline_expressions.pals.yaml diff --git a/tests/test_include.py b/tests/test_include.py index 19d46f2..bf011b7 100644 --- a/tests/test_include.py +++ b/tests/test_include.py @@ -1,143 +1,248 @@ -import pals +"""Tests for resolving `include` entries while loading files.""" +import textwrap -def test_include(tmp_path): - main_file = tmp_path / "main.pals.yaml" - root_included_file = tmp_path / "included.pals.yaml" - facility_included_file = tmp_path / "facility.pals.yaml" - facility_nested_file = tmp_path / "facility_nested.pals.yaml" +import pytest - main_content = f""" - PALS: - include: "{root_included_file.name}" - facility: - - drift1: - kind: Drift - length: 0.25 +import pals - - include: "{facility_included_file.name}" - - fodo_cell: - kind: BeamLine - line: - - drift1 - - quad1 - - drift2 - - quad2 - - drift1 - - - fodo_lattice: - kind: Lattice - branches: - - fodo_cell - - - use: fodo_lattice - """ +def write(path, text): + """Write a dedented YAML document, creating parent directories.""" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(textwrap.dedent(text)) + return path - root_included_content = """ - author: "Some One " - version: 1.0 - """ - facility_included_content = f""" - - quad1: - kind: Quadrupole - MagneticMultipoleP: - Bn1: 1.0 - length: 1.0 - - - drift2: - kind: Drift - length: 0.5 - - - include: "{facility_nested_file.name}" - """ +def test_include(tmp_path): + """Includes splice into the root node and the facility list.""" + main_file = write( + tmp_path / "main.pals.yaml", + """\ + PALS: + include: "globals.subpals.yaml" + facility: + - drift1: + kind: Drift + length: 0.25 + + - include: "sub/quads.subpals.yaml" + + - fodo_cell: + kind: BeamLine + line: + - drift1 + - quad1 + - drift2 + - quad2 + - drift1 + + - fodo_lattice: + kind: Lattice + branches: + - fodo_cell + + - use: fodo_lattice + """, + ) + # Root-level include: version and notes per the standard's example, plus + # an unmodeled key that must be preserved. + write( + tmp_path / "globals.subpals.yaml", + """\ + version: 1.0 + notes: + - "included note" + my_extension_data: "kept" + """, + ) + # Facility-level include, itself including a file relative to its own + # directory (not the directory of the main file). + write( + tmp_path / "sub" / "quads.subpals.yaml", + """\ + - quad1: + kind: Quadrupole + MagneticMultipoleP: + Bn1: 1.0 + length: 1.0 - facility_nested_content = """ + - drift2: + kind: Drift + length: 0.5 + + - include: "../parts/extra.subpals.yaml" + """, + ) + write( + tmp_path / "parts" / "extra.subpals.yaml", + """\ - quad2: kind: Quadrupole MagneticMultipoleP: Bn1: -1.0 length: 1.0 - """ - - main_file.write_text(main_content) - root_included_file.write_text(root_included_content) - facility_included_file.write_text(facility_included_content) - facility_nested_file.write_text(facility_nested_content) + """, + ) lattice = pals.Lattice.from_file(main_file) - assert lattice.name == "fodo_lattice" assert lattice.branches[0] == "fodo_cell" data = pals.PALSroot.from_file(main_file) - assert data.version == 1.0 - assert data.author == "Some One " + assert data.notes == ["included note"] + assert data.my_extension_data == "kept" assert not hasattr(data, "include") + names = [elem.name for elem in data.facility] + assert names == [ + "drift1", + "quad1", + "drift2", + "quad2", + "fodo_cell", + "fodo_lattice", + "fodo_lattice", + ] + assert isinstance(data.facility[3], pals.Quadrupole) + assert data.facility[3].MagneticMultipoleP.Bn1 == -1.0 -def test_nested_include(tmp_path): - root_file = tmp_path / "root.pals.yaml" - middle_file = tmp_path / "middle.pals.yaml" - leaf_file = tmp_path / "leaf.pals.yaml" - - root_content = f""" - root: - include: "{middle_file.name}" - """ - middle_content = f""" - middle: val - include: "{leaf_file.name}" - """ - - leaf_content = """ - leaf: val - """ - - root_file.write_text(root_content) - middle_file.write_text(middle_content) - leaf_file.write_text(leaf_content) +def test_nested_include(tmp_path): + """A chain of includes resolves through each file; local keys win.""" + root_file = write( + tmp_path / "root.pals.yaml", + """\ + root: + include: "middle.subpals.yaml" + """, + ) + write( + tmp_path / "middle.subpals.yaml", + """\ + middle: val + shared: local + include: "leaf.subpals.yaml" + """, + ) + write( + tmp_path / "leaf.subpals.yaml", + """\ + leaf: val + shared: included + """, + ) data = pals.functions.load_file_to_dict(str(root_file)) assert data["root"]["middle"] == "val" assert data["root"]["leaf"] == "val" + # The including file's own entry wins over the included one. + assert data["root"]["shared"] == "local" assert "include" not in data["root"] -def test_include_list_into_dict_conversion(tmp_path): - # This tests the spec example where a list of properties is included into a dict (element) - main_file = tmp_path / "element.pals.yaml" - params_file = tmp_path / "params.pals.yaml" - - main_content = f""" - element: - kind: Quadrupole - include: "{params_file.name}" - """ - - # params file content is a list of single-key dicts - params_content = """ - - MagneticMultipoleP: - - Kn3L: 0.3 - - ApertureP: - x_limits: [-0.1, 0.1] - """ - - main_file.write_text(main_content) - params_file.write_text(params_content) +def test_include_element_parameters(tmp_path): + """The standard's element-level include example: a parameter group file.""" + main_file = write( + tmp_path / "element.pals.yaml", + """\ + Q01: + kind: Quadrupole + include: "include-Q-params.subpals.yaml" + """, + ) + write( + tmp_path / "include-Q-params.subpals.yaml", + """\ + MagneticMultipoleP: + Kn3L: 0.3 + """, + ) data = pals.functions.load_file_to_dict(str(main_file)) - - elem = data["element"] - assert elem["kind"] == "Quadrupole" - - # Check if keys are correctly merged from the list - assert "MagneticMultipoleP" in elem - assert elem["MagneticMultipoleP"] == [{"Kn3L": 0.3}] - - assert "ApertureP" in elem - assert elem["ApertureP"] == {"x_limits": [-0.1, 0.1]} + assert data["Q01"] == { + "kind": "Quadrupole", + "MagneticMultipoleP": {"Kn3L": 0.3}, + } + + +def test_include_structure_mismatch(tmp_path): + """Including a sequence at a mapping level is a structural error.""" + main_file = write( + tmp_path / "element.pals.yaml", + """\ + Q01: + kind: Quadrupole + include: "elements.subpals.yaml" + """, + ) + write( + tmp_path / "elements.subpals.yaml", + """\ + - a: + kind: Drift + """, + ) + + with pytest.raises(TypeError, match="mapping level"): + pals.functions.load_file_to_dict(str(main_file)) + + +def test_circular_include(tmp_path): + """An include cycle is reported instead of recursing forever.""" + a_file = write( + tmp_path / "a.pals.yaml", + """\ + PALS: + include: "b.subpals.yaml" + """, + ) + write( + tmp_path / "b.subpals.yaml", + """\ + include: "a.pals.yaml" + """, + ) + + with pytest.raises(RuntimeError, match="circular include"): + pals.functions.load_file_to_dict(str(a_file)) + + +def test_lattice_from_full_document_use(tmp_path): + """Per the use statement, the last lattice is instantiated unless a + `use` entry selects another.""" + content = """\ + PALS: + facility: + - line1: + kind: BeamLine + line: + - m1: + kind: Marker + - lat1: + kind: Lattice + branches: + - line1 + - lat2: + kind: Lattice + branches: + - line1 + """ + no_use = write(tmp_path / "no_use.pals.yaml", content) + assert pals.Lattice.from_file(no_use).name == "lat2" + + with_use = write(tmp_path / "with_use.pals.yaml", content + ' - use: "lat1"\n') + assert pals.Lattice.from_file(with_use).name == "lat1" + + no_lattice = write( + tmp_path / "no_lattice.pals.yaml", + """\ + PALS: + notes: + - "no facility here" + """, + ) + with pytest.raises(ValueError, match="does not define a Lattice"): + pals.Lattice.from_file(no_lattice) From d64973963153cc42f85554b9ec37fe9df0bd5464 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Thu, 30 Jul 2026 16:13:13 -0700 Subject: [PATCH 5/6] Move include test documents into tests/pals_files The YAML documents the include tests exercise are checked-in files under tests/pals_files (one directory per scenario) instead of literal strings written to tmp_path, mirroring how the standard organizes its examples corpus. Relative include targets now demonstrably resolve against the including file's checked-in location. Co-Authored-By: Claude Fable 5 --- tests/pals_files/include/circular/a.pals.yaml | 3 + .../include/circular/b.subpals.yaml | 2 + .../element/include-Q-params.subpals.yaml | 3 + .../pals_files/include/element/q01.pals.yaml | 5 + tests/pals_files/include/globals.subpals.yaml | 6 + tests/pals_files/include/main.pals.yaml | 28 +++ .../include/mismatch/elements.subpals.yaml | 3 + .../pals_files/include/mismatch/q01.pals.yaml | 4 + .../include/nested/leaf.subpals.yaml | 3 + .../include/nested/middle.subpals.yaml | 5 + .../pals_files/include/nested/root.pals.yaml | 5 + .../include/parts/extra.subpals.yaml | 6 + .../pals_files/include/sub/quads.subpals.yaml | 13 ++ .../lattice_use/no_lattice.pals.yaml | 4 + tests/pals_files/lattice_use/no_use.pals.yaml | 17 ++ .../pals_files/lattice_use/with_use.pals.yaml | 18 ++ tests/test_include.py | 219 +++--------------- 17 files changed, 157 insertions(+), 187 deletions(-) create mode 100644 tests/pals_files/include/circular/a.pals.yaml create mode 100644 tests/pals_files/include/circular/b.subpals.yaml create mode 100644 tests/pals_files/include/element/include-Q-params.subpals.yaml create mode 100644 tests/pals_files/include/element/q01.pals.yaml create mode 100644 tests/pals_files/include/globals.subpals.yaml create mode 100644 tests/pals_files/include/main.pals.yaml create mode 100644 tests/pals_files/include/mismatch/elements.subpals.yaml create mode 100644 tests/pals_files/include/mismatch/q01.pals.yaml create mode 100644 tests/pals_files/include/nested/leaf.subpals.yaml create mode 100644 tests/pals_files/include/nested/middle.subpals.yaml create mode 100644 tests/pals_files/include/nested/root.pals.yaml create mode 100644 tests/pals_files/include/parts/extra.subpals.yaml create mode 100644 tests/pals_files/include/sub/quads.subpals.yaml create mode 100644 tests/pals_files/lattice_use/no_lattice.pals.yaml create mode 100644 tests/pals_files/lattice_use/no_use.pals.yaml create mode 100644 tests/pals_files/lattice_use/with_use.pals.yaml diff --git a/tests/pals_files/include/circular/a.pals.yaml b/tests/pals_files/include/circular/a.pals.yaml new file mode 100644 index 0000000..2678f53 --- /dev/null +++ b/tests/pals_files/include/circular/a.pals.yaml @@ -0,0 +1,3 @@ +# Includes b.subpals.yaml, which includes this file again: a cycle. +PALS: + include: "b.subpals.yaml" diff --git a/tests/pals_files/include/circular/b.subpals.yaml b/tests/pals_files/include/circular/b.subpals.yaml new file mode 100644 index 0000000..3db2753 --- /dev/null +++ b/tests/pals_files/include/circular/b.subpals.yaml @@ -0,0 +1,2 @@ +# Includes a.pals.yaml, closing the include cycle. +include: "a.pals.yaml" diff --git a/tests/pals_files/include/element/include-Q-params.subpals.yaml b/tests/pals_files/include/element/include-Q-params.subpals.yaml new file mode 100644 index 0000000..da04131 --- /dev/null +++ b/tests/pals_files/include/element/include-Q-params.subpals.yaml @@ -0,0 +1,3 @@ +# Included into q01.pals.yaml, from the standard's include example. +MagneticMultipoleP: + Kn3L: 0.3 diff --git a/tests/pals_files/include/element/q01.pals.yaml b/tests/pals_files/include/element/q01.pals.yaml new file mode 100644 index 0000000..1061e52 --- /dev/null +++ b/tests/pals_files/include/element/q01.pals.yaml @@ -0,0 +1,5 @@ +# The standard's element-level include example: the element pulls a +# parameter group in from include-Q-params.subpals.yaml. +Q01: + kind: Quadrupole + include: "include-Q-params.subpals.yaml" diff --git a/tests/pals_files/include/globals.subpals.yaml b/tests/pals_files/include/globals.subpals.yaml new file mode 100644 index 0000000..8d89b3b --- /dev/null +++ b/tests/pals_files/include/globals.subpals.yaml @@ -0,0 +1,6 @@ +# Included at the root level of main.pals.yaml: version and notes per the +# standard's example, plus an unmodeled key that must be preserved. +version: 1.0 +notes: + - "included note" +my_extension_data: "kept" diff --git a/tests/pals_files/include/main.pals.yaml b/tests/pals_files/include/main.pals.yaml new file mode 100644 index 0000000..7d6db4d --- /dev/null +++ b/tests/pals_files/include/main.pals.yaml @@ -0,0 +1,28 @@ +# A FODO document assembled with includes: a root-level include contributes +# document metadata (globals.subpals.yaml) and a facility-level include +# splices in element definitions (sub/quads.subpals.yaml, which itself +# includes parts/extra.subpals.yaml relative to its own directory). +PALS: + include: "globals.subpals.yaml" + facility: + - drift1: + kind: Drift + length: 0.25 + + - include: "sub/quads.subpals.yaml" + + - fodo_cell: + kind: BeamLine + line: + - drift1 + - quad1 + - drift2 + - quad2 + - drift1 + + - fodo_lattice: + kind: Lattice + branches: + - fodo_cell + + - use: fodo_lattice diff --git a/tests/pals_files/include/mismatch/elements.subpals.yaml b/tests/pals_files/include/mismatch/elements.subpals.yaml new file mode 100644 index 0000000..8ee321f --- /dev/null +++ b/tests/pals_files/include/mismatch/elements.subpals.yaml @@ -0,0 +1,3 @@ +# A sequence, included by q01.pals.yaml at a mapping level: an error. +- a: + kind: Drift diff --git a/tests/pals_files/include/mismatch/q01.pals.yaml b/tests/pals_files/include/mismatch/q01.pals.yaml new file mode 100644 index 0000000..f6fb04d --- /dev/null +++ b/tests/pals_files/include/mismatch/q01.pals.yaml @@ -0,0 +1,4 @@ +# Structural error: includes a file holding a sequence at a mapping level. +Q01: + kind: Quadrupole + include: "elements.subpals.yaml" diff --git a/tests/pals_files/include/nested/leaf.subpals.yaml b/tests/pals_files/include/nested/leaf.subpals.yaml new file mode 100644 index 0000000..06a5388 --- /dev/null +++ b/tests/pals_files/include/nested/leaf.subpals.yaml @@ -0,0 +1,3 @@ +# Included by middle.subpals.yaml; its `shared` entry is overridden. +leaf: val +shared: included diff --git a/tests/pals_files/include/nested/middle.subpals.yaml b/tests/pals_files/include/nested/middle.subpals.yaml new file mode 100644 index 0000000..4f63da2 --- /dev/null +++ b/tests/pals_files/include/nested/middle.subpals.yaml @@ -0,0 +1,5 @@ +# Included by root.pals.yaml; includes leaf.subpals.yaml itself. Its own +# `shared` entry wins over the one from the leaf. +middle: val +shared: local +include: "leaf.subpals.yaml" diff --git a/tests/pals_files/include/nested/root.pals.yaml b/tests/pals_files/include/nested/root.pals.yaml new file mode 100644 index 0000000..86f242a --- /dev/null +++ b/tests/pals_files/include/nested/root.pals.yaml @@ -0,0 +1,5 @@ +# A chain of includes: this file includes middle.subpals.yaml, which +# includes leaf.subpals.yaml. Keys local to an including file win over +# included ones. +root: + include: "middle.subpals.yaml" diff --git a/tests/pals_files/include/parts/extra.subpals.yaml b/tests/pals_files/include/parts/extra.subpals.yaml new file mode 100644 index 0000000..1fa547d --- /dev/null +++ b/tests/pals_files/include/parts/extra.subpals.yaml @@ -0,0 +1,6 @@ +# Spliced into sub/quads.subpals.yaml by its include entry. +- quad2: + kind: Quadrupole + MagneticMultipoleP: + Bn1: -1.0 + length: 1.0 diff --git a/tests/pals_files/include/sub/quads.subpals.yaml b/tests/pals_files/include/sub/quads.subpals.yaml new file mode 100644 index 0000000..78a1576 --- /dev/null +++ b/tests/pals_files/include/sub/quads.subpals.yaml @@ -0,0 +1,13 @@ +# Spliced into the facility of main.pals.yaml; includes a further file +# relative to its own directory (not the directory of the main file). +- quad1: + kind: Quadrupole + MagneticMultipoleP: + Bn1: 1.0 + length: 1.0 + +- drift2: + kind: Drift + length: 0.5 + +- include: "../parts/extra.subpals.yaml" diff --git a/tests/pals_files/lattice_use/no_lattice.pals.yaml b/tests/pals_files/lattice_use/no_lattice.pals.yaml new file mode 100644 index 0000000..bd21b3a --- /dev/null +++ b/tests/pals_files/lattice_use/no_lattice.pals.yaml @@ -0,0 +1,4 @@ +# A document defining no Lattice: Lattice.from_file reports an error. +PALS: + notes: + - "no facility here" diff --git a/tests/pals_files/lattice_use/no_use.pals.yaml b/tests/pals_files/lattice_use/no_use.pals.yaml new file mode 100644 index 0000000..49fc108 --- /dev/null +++ b/tests/pals_files/lattice_use/no_use.pals.yaml @@ -0,0 +1,17 @@ +# Two lattices and no use entry: per the standard's use statement, the +# last lattice defined (lat2) is the one instantiated. +PALS: + facility: + - line1: + kind: BeamLine + line: + - m1: + kind: Marker + - lat1: + kind: Lattice + branches: + - line1 + - lat2: + kind: Lattice + branches: + - line1 diff --git a/tests/pals_files/lattice_use/with_use.pals.yaml b/tests/pals_files/lattice_use/with_use.pals.yaml new file mode 100644 index 0000000..058a364 --- /dev/null +++ b/tests/pals_files/lattice_use/with_use.pals.yaml @@ -0,0 +1,18 @@ +# Two lattices and a use entry: use overrides the last-lattice default, +# selecting lat1. +PALS: + facility: + - line1: + kind: BeamLine + line: + - m1: + kind: Marker + - lat1: + kind: Lattice + branches: + - line1 + - lat2: + kind: Lattice + branches: + - line1 + - use: "lat1" diff --git a/tests/test_include.py b/tests/test_include.py index bf011b7..3a64d7c 100644 --- a/tests/test_include.py +++ b/tests/test_include.py @@ -1,100 +1,38 @@ -"""Tests for resolving `include` entries while loading files.""" +"""Tests for resolving `include` entries while loading files. -import textwrap +The documents used here live under tests/pals_files/include and +tests/pals_files/lattice_use, one directory per scenario. +""" + +import pathlib import pytest import pals - -def write(path, text): - """Write a dedented YAML document, creating parent directories.""" - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(textwrap.dedent(text)) - return path +PALS_FILES = pathlib.Path(__file__).parent / "pals_files" +INCLUDE = PALS_FILES / "include" +LATTICE_USE = PALS_FILES / "lattice_use" -def test_include(tmp_path): +def test_include(): """Includes splice into the root node and the facility list.""" - main_file = write( - tmp_path / "main.pals.yaml", - """\ - PALS: - include: "globals.subpals.yaml" - facility: - - drift1: - kind: Drift - length: 0.25 - - - include: "sub/quads.subpals.yaml" - - - fodo_cell: - kind: BeamLine - line: - - drift1 - - quad1 - - drift2 - - quad2 - - drift1 - - - fodo_lattice: - kind: Lattice - branches: - - fodo_cell - - - use: fodo_lattice - """, - ) - # Root-level include: version and notes per the standard's example, plus - # an unmodeled key that must be preserved. - write( - tmp_path / "globals.subpals.yaml", - """\ - version: 1.0 - notes: - - "included note" - my_extension_data: "kept" - """, - ) - # Facility-level include, itself including a file relative to its own - # directory (not the directory of the main file). - write( - tmp_path / "sub" / "quads.subpals.yaml", - """\ - - quad1: - kind: Quadrupole - MagneticMultipoleP: - Bn1: 1.0 - length: 1.0 - - - drift2: - kind: Drift - length: 0.5 - - - include: "../parts/extra.subpals.yaml" - """, - ) - write( - tmp_path / "parts" / "extra.subpals.yaml", - """\ - - quad2: - kind: Quadrupole - MagneticMultipoleP: - Bn1: -1.0 - length: 1.0 - """, - ) + main_file = str(INCLUDE / "main.pals.yaml") lattice = pals.Lattice.from_file(main_file) assert lattice.name == "fodo_lattice" assert lattice.branches[0] == "fodo_cell" data = pals.PALSroot.from_file(main_file) + # From the root-level include of globals.subpals.yaml, with the + # unmodeled key preserved. assert data.version == 1.0 assert data.notes == ["included note"] assert data.my_extension_data == "kept" assert not hasattr(data, "include") + # sub/quads.subpals.yaml spliced in quad1 and drift2, and its own + # include (resolved relative to sub/) spliced in quad2. names = [elem.name for elem in data.facility] assert names == [ "drift1", @@ -109,32 +47,9 @@ def test_include(tmp_path): assert data.facility[3].MagneticMultipoleP.Bn1 == -1.0 -def test_nested_include(tmp_path): +def test_nested_include(): """A chain of includes resolves through each file; local keys win.""" - root_file = write( - tmp_path / "root.pals.yaml", - """\ - root: - include: "middle.subpals.yaml" - """, - ) - write( - tmp_path / "middle.subpals.yaml", - """\ - middle: val - shared: local - include: "leaf.subpals.yaml" - """, - ) - write( - tmp_path / "leaf.subpals.yaml", - """\ - leaf: val - shared: included - """, - ) - - data = pals.functions.load_file_to_dict(str(root_file)) + data = pals.functions.load_file_to_dict(str(INCLUDE / "nested/root.pals.yaml")) assert data["root"]["middle"] == "val" assert data["root"]["leaf"] == "val" @@ -143,106 +58,36 @@ def test_nested_include(tmp_path): assert "include" not in data["root"] -def test_include_element_parameters(tmp_path): +def test_include_element_parameters(): """The standard's element-level include example: a parameter group file.""" - main_file = write( - tmp_path / "element.pals.yaml", - """\ - Q01: - kind: Quadrupole - include: "include-Q-params.subpals.yaml" - """, - ) - write( - tmp_path / "include-Q-params.subpals.yaml", - """\ - MagneticMultipoleP: - Kn3L: 0.3 - """, - ) - - data = pals.functions.load_file_to_dict(str(main_file)) + data = pals.functions.load_file_to_dict(str(INCLUDE / "element/q01.pals.yaml")) + assert data["Q01"] == { "kind": "Quadrupole", "MagneticMultipoleP": {"Kn3L": 0.3}, } -def test_include_structure_mismatch(tmp_path): +def test_include_structure_mismatch(): """Including a sequence at a mapping level is a structural error.""" - main_file = write( - tmp_path / "element.pals.yaml", - """\ - Q01: - kind: Quadrupole - include: "elements.subpals.yaml" - """, - ) - write( - tmp_path / "elements.subpals.yaml", - """\ - - a: - kind: Drift - """, - ) - with pytest.raises(TypeError, match="mapping level"): - pals.functions.load_file_to_dict(str(main_file)) + pals.functions.load_file_to_dict(str(INCLUDE / "mismatch/q01.pals.yaml")) -def test_circular_include(tmp_path): +def test_circular_include(): """An include cycle is reported instead of recursing forever.""" - a_file = write( - tmp_path / "a.pals.yaml", - """\ - PALS: - include: "b.subpals.yaml" - """, - ) - write( - tmp_path / "b.subpals.yaml", - """\ - include: "a.pals.yaml" - """, - ) - with pytest.raises(RuntimeError, match="circular include"): - pals.functions.load_file_to_dict(str(a_file)) + pals.functions.load_file_to_dict(str(INCLUDE / "circular/a.pals.yaml")) -def test_lattice_from_full_document_use(tmp_path): +def test_lattice_from_full_document_use(): """Per the use statement, the last lattice is instantiated unless a `use` entry selects another.""" - content = """\ - PALS: - facility: - - line1: - kind: BeamLine - line: - - m1: - kind: Marker - - lat1: - kind: Lattice - branches: - - line1 - - lat2: - kind: Lattice - branches: - - line1 - """ - no_use = write(tmp_path / "no_use.pals.yaml", content) - assert pals.Lattice.from_file(no_use).name == "lat2" - - with_use = write(tmp_path / "with_use.pals.yaml", content + ' - use: "lat1"\n') - assert pals.Lattice.from_file(with_use).name == "lat1" - - no_lattice = write( - tmp_path / "no_lattice.pals.yaml", - """\ - PALS: - notes: - - "no facility here" - """, - ) + lattice = pals.Lattice.from_file(str(LATTICE_USE / "no_use.pals.yaml")) + assert lattice.name == "lat2" + + lattice = pals.Lattice.from_file(str(LATTICE_USE / "with_use.pals.yaml")) + assert lattice.name == "lat1" + with pytest.raises(ValueError, match="does not define a Lattice"): - pals.Lattice.from_file(no_lattice) + pals.Lattice.from_file(str(LATTICE_USE / "no_lattice.pals.yaml")) From 81c8a7f0818dc5902e17a51e323ab23cdd356e14 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Thu, 30 Jul 2026 16:49:09 -0700 Subject: [PATCH 6/6] Address review: use validation, include scoping, symlinked cycles - A `use` entry naming no defined Lattice is an error in Lattice.from_file instead of a silent fall-through to the last lattice: per the standard, use overrides the default selection. PlaceholderName now records whether a reference was written as a `use:` entry, which also lets such entries keep their form when serialized instead of degrading to bare name references. - Include resolution in a full document is scoped to the PALS root node, where the standard requires include statements to live; top-level siblings of the PALS node are outside the standard and are no longer traversed (an unresolvable include there no longer errors). - Include cycle detection canonicalizes paths with realpath so a cycle routed through a directory symlink is reported as circular instead of recursing into the filesystem limit. Co-Authored-By: Claude Fable 5 --- src/pals/functions.py | 19 +++++++---- src/pals/kinds/Lattice.py | 28 ++++++++++------ src/pals/kinds/PlaceholderName.py | 11 +++++-- src/pals/kinds/mixin/all_element_mixin.py | 2 +- .../include/outside/globals.subpals.yaml | 2 ++ .../pals_files/include/outside/main.pals.yaml | 9 ++++++ .../pals_files/lattice_use/bad_use.pals.yaml | 18 +++++++++++ tests/test_include.py | 32 +++++++++++++++++++ 8 files changed, 102 insertions(+), 19 deletions(-) create mode 100644 tests/pals_files/include/outside/globals.subpals.yaml create mode 100644 tests/pals_files/include/outside/main.pals.yaml create mode 100644 tests/pals_files/lattice_use/bad_use.pals.yaml diff --git a/src/pals/functions.py b/src/pals/functions.py index daa9238..91b3649 100644 --- a/src/pals/functions.py +++ b/src/pals/functions.py @@ -113,7 +113,8 @@ def load_file_to_dict( filename: str, sub_level: bool = False, _include_chain: tuple = () ) -> dict: # Guard against include cycles: a file including itself through any chain. - filepath = os.path.abspath(filename) + # realpath canonicalizes symlinks so a cycle cannot hide behind one. + filepath = os.path.realpath(filename) if filepath in _include_chain: chain = " -> ".join(_include_chain + (filepath,)) raise RuntimeError(f"load_file_to_dict: circular include: {chain}") @@ -143,12 +144,16 @@ def load_file_to_dict( f"load_file_to_dict: No support for PALS file {filename} with extension {extension} yet." ) - # Resolve include entries, tracking this file for cycle detection - pals_data = process_includes( - pals_data, - base_dir=os.path.dirname(filename), - include_chain=_include_chain + (filepath,), - ) + # Resolve include entries, tracking this file for cycle detection. In a + # full document, include statements must be within the PALS root node; + # information outside of it is outside the standard and is not touched. + base_dir = os.path.dirname(filename) + include_chain = _include_chain + (filepath,) + if isinstance(pals_data, dict) and "PALS" in pals_data: + pals_data = dict(pals_data) + pals_data["PALS"] = process_includes(pals_data["PALS"], base_dir, include_chain) + else: + pals_data = process_includes(pals_data, base_dir, include_chain) return pals_data diff --git a/src/pals/kinds/Lattice.py b/src/pals/kinds/Lattice.py index 71845c5..61e7276 100644 --- a/src/pals/kinds/Lattice.py +++ b/src/pals/kinds/Lattice.py @@ -45,19 +45,29 @@ def from_file(filename: str) -> Self: pals_root = PALSroot(**pals_dict) facility = pals_root.facility or [] lattices = [item for item in facility if isinstance(item, Lattice)] + by_name = {lattice.name: lattice for lattice in lattices} + + # A `use` entry overrides the last-lattice default; with several, + # the last one wins. It must name a Lattice the document defines. + use_entries = [ + item + for item in facility + if isinstance(item, PlaceholderName) and item.is_use + ] + if use_entries: + selected = use_entries[-1].name + if selected not in by_name: + raise ValueError( + f"PALS root document {filename!r} selects {selected!r} " + f"with its use entry, but defines no Lattice of that " + f"name; defined Lattices: {sorted(by_name)}" + ) + return by_name[selected] + if not lattices: raise ValueError( f"PALS root document {filename!r} does not define a Lattice" ) - by_name = {lattice.name: lattice for lattice in lattices} - - # `use` entries are stored as name references; the last one naming - # a defined Lattice wins. References to non-Lattice elements - # (e.g. facility-level commands) do not select anything. - for item in reversed(facility): - if isinstance(item, PlaceholderName) and item.name in by_name: - return by_name[item.name] - return lattices[-1] return Lattice(**pals_dict) diff --git a/src/pals/kinds/PlaceholderName.py b/src/pals/kinds/PlaceholderName.py index c9b7c96..aef0ebf 100644 --- a/src/pals/kinds/PlaceholderName.py +++ b/src/pals/kinds/PlaceholderName.py @@ -41,14 +41,21 @@ class PlaceholderName(BaseModel): "BaseElement | None", Field(default=None, description="Reference to the resolved element object"), ] = None + is_use: bool = Field( + default=False, + description="True when this reference was written as a `use:` entry", + ) @model_serializer(mode="plain") - def _serialize_as_name(self) -> str: - """Serialize this reference as just its name. + def _serialize_as_name(self) -> str | dict[str, str]: + """Serialize this reference as its name, or its `use:` entry form. This makes `model_dump()` return a string (the element name), so nested serialization (e.g. inside BeamLine.line) produces plain strings too. + References written as `use:` entries keep that form. """ + if self.is_use: + return {"use": self.name} return self.name def __init__(self, name: str | None = None, /, **data): diff --git a/src/pals/kinds/mixin/all_element_mixin.py b/src/pals/kinds/mixin/all_element_mixin.py index d67e5c8..3b40795 100644 --- a/src/pals/kinds/mixin/all_element_mixin.py +++ b/src/pals/kinds/mixin/all_element_mixin.py @@ -51,7 +51,7 @@ def unpack_element_items(items: list, container_type: str) -> list: # can resolve it. if not isinstance(fields, dict): if name == "use" and isinstance(fields, str): - new_list.append(PlaceholderName(fields)) + new_list.append(PlaceholderName(fields, is_use=True)) continue raise TypeError( f"Value for element key {name!r} must be a dict (the element's properties), " diff --git a/tests/pals_files/include/outside/globals.subpals.yaml b/tests/pals_files/include/outside/globals.subpals.yaml new file mode 100644 index 0000000..0518df1 --- /dev/null +++ b/tests/pals_files/include/outside/globals.subpals.yaml @@ -0,0 +1,2 @@ +# Included within the PALS node of main.pals.yaml. +version: "1.0" diff --git a/tests/pals_files/include/outside/main.pals.yaml b/tests/pals_files/include/outside/main.pals.yaml new file mode 100644 index 0000000..1a4a788 --- /dev/null +++ b/tests/pals_files/include/outside/main.pals.yaml @@ -0,0 +1,9 @@ +# Information outside the PALS root node is outside the standard and is +# ignored: the top-level include names a file that does not exist and must +# not be resolved. The include inside the PALS node is. +include: "missing.subpals.yaml" +not_pals_data: true +PALS: + include: "globals.subpals.yaml" + notes: + - "the PALS node itself" diff --git a/tests/pals_files/lattice_use/bad_use.pals.yaml b/tests/pals_files/lattice_use/bad_use.pals.yaml new file mode 100644 index 0000000..5fd9bc3 --- /dev/null +++ b/tests/pals_files/lattice_use/bad_use.pals.yaml @@ -0,0 +1,18 @@ +# A use entry naming no defined Lattice: since use overrides the default +# lattice selection, this is an error, not a fallback to the last lattice. +PALS: + facility: + - line1: + kind: BeamLine + line: + - m1: + kind: Marker + - lat1: + kind: Lattice + branches: + - line1 + - lat2: + kind: Lattice + branches: + - line1 + - use: "typo" diff --git a/tests/test_include.py b/tests/test_include.py index 3a64d7c..bf7f6c3 100644 --- a/tests/test_include.py +++ b/tests/test_include.py @@ -74,12 +74,39 @@ def test_include_structure_mismatch(): pals.functions.load_file_to_dict(str(INCLUDE / "mismatch/q01.pals.yaml")) +def test_include_outside_pals_node(): + """Include statements must be within the PALS root node; the top-level + sibling include naming a missing file is outside the standard and is + left untouched.""" + data = pals.functions.load_file_to_dict(str(INCLUDE / "outside/main.pals.yaml")) + assert data["include"] == "missing.subpals.yaml" + assert data["PALS"]["version"] == "1.0" + + root = pals.load(str(INCLUDE / "outside/main.pals.yaml")) + assert root.version == "1.0" + assert root.notes == ["the PALS node itself"] + + def test_circular_include(): """An include cycle is reported instead of recursing forever.""" with pytest.raises(RuntimeError, match="circular include"): pals.functions.load_file_to_dict(str(INCLUDE / "circular/a.pals.yaml")) +def test_symlinked_circular_include(tmp_path): + """A cycle routed through a directory symlink is still detected.""" + link = tmp_path / "link" + try: + link.symlink_to(tmp_path, target_is_directory=True) + except OSError: + pytest.skip("platform cannot create symlinks") + root_file = tmp_path / "root.pals.yaml" + root_file.write_text('PALS:\n include: "link/root.pals.yaml"\n') + + with pytest.raises(RuntimeError, match="circular include"): + pals.functions.load_file_to_dict(str(root_file)) + + def test_lattice_from_full_document_use(): """Per the use statement, the last lattice is instantiated unless a `use` entry selects another.""" @@ -91,3 +118,8 @@ def test_lattice_from_full_document_use(): with pytest.raises(ValueError, match="does not define a Lattice"): pals.Lattice.from_file(str(LATTICE_USE / "no_lattice.pals.yaml")) + + # A use entry that names no defined Lattice errors instead of silently + # falling back to the last lattice. + with pytest.raises(ValueError, match="no Lattice of that name"): + pals.Lattice.from_file(str(LATTICE_USE / "bad_use.pals.yaml"))