gdsfactory / gdsfactory

python library to design chips (Photonics, Analog, Quantum, MEMs, ...), objects for 3D printing or PCBs.
https://gdsfactory.github.io/gdsfactory/
MIT License
536 stars 222 forks source link

Importing gdsfactory changes `yaml.safe_load` behavior #3188

Closed 95-martin-orion closed 2 weeks ago

95-martin-orion commented 2 months ago

Describe the bug Importing gdsfactory changes the behavior of yaml such that YAML sequences are converted to tuples instead of lists when fed into yaml.safe_load.

To Reproduce

import pathlib

def test_yaml(tmp_path: pathlib.Path):
    import yaml
    tmp_file = tmp_path / "foo.yml"

    with tmp_file.open("w") as stream:
        yaml.dump([1, 2, 3], stream)
    with tmp_file.open("r") as stream:
        assert yaml.safe_load(stream) == [1, 2, 3]

    import gdsfactory as gf
    tmp_file2 = tmp_path / "foo2.yml"

    with tmp_file2.open("w") as stream:
        yaml.dump([1, 2, 3], stream)
    with tmp_file2.open("r") as stream:
        assert yaml.safe_load(stream) == [1, 2, 3]

test_yaml(pathlib.Path("/tmp")
# AssertionError ( yaml.safe_load(stream) != [1, 2, 3] )

Expected behavior Importing gdsfactory should not change the behavior of yaml.

Suggested fix The root cause of this issue is here: https://github.com/gdsfactory/gdsfactory/blob/afc67aabbb58dbf550dc956506e277d8f7ac7586/gdsfactory/read/from_yaml.py#L764-L773

Making this behavior apply to a custom YAML loader instead of yaml.SafeLoader should fix the issue.

Environment (please complete the following information):

Version info ```python import sys print(sys.version) # 3.11.8 (main, Mar 27 2024, 18:30:28) [GCC 12.2.0] print(sys.executable) # [redacted]/bin/python import gdsfactory as gf gf.config.print_version_plugins() Modules ┏━━━━━━━━━━━━┳━━━━━━━━━━━┳━━━━━━━━━━━┓ ┃ Package ┃ version ┃ Path ┃ ┡━━━━━━━━━━━━╇━━━━━━━━━━━╇━━━━━━━━━━━┩ │ python │ 3.11.8 │ /usr/loc… │ │ │ (main, │ │ │ │ Mar 27 │ │ │ │ 2024, │ │ │ │ 18:30:28) │ │ │ │ [GCC │ │ │ │ 12.2.0] │ │ │ gdsfactory │ 8.8.2 │ /usr/loc… │ │ gplugins │ not │ │ │ │ installed │ │ │ ray │ not │ │ │ │ installed │ │ │ femwell │ not │ │ │ │ installed │ │ │ devsim │ not │ │ │ │ installed │ │ │ tidy3d │ not │ │ │ │ installed │ │ │ meep │ not │ │ │ │ installed │ │ │ meow │ not │ │ │ │ installed │ │ │ lumapi │ not │ │ │ │ installed │ │ │ sax │ not │ │ │ │ installed │ │ └────────────┴───────────┴───────────┘ ```
95-martin-orion commented 1 month ago

More instances of yaml mutation can be found here. These are less problematic as they only happen when the functions are called, not at import time, but they can still cause issues in projects depending on gdsfactory.

If a downstream project previously expected yaml to have its default behavior, it's possible in theory to wrap those yaml calls in a helper which temporarily restores the previous behavior (see safe_load example below), but doing so is a fairly invasive change for any project with widespread use of yaml.

Sample safe_load wrapper ```py def yaml_safe_load(stream): try: constructor_backup = yaml.SafeLoader.yaml_constructors[ yaml.resolver.BaseResolver.DEFAULT_SEQUENCE_TAG ] yaml.add_constructor( yaml.resolver.BaseResolver.DEFAULT_SEQUENCE_TAG, yaml.constructor.SafeConstructor.construct_sequence, Loader=yaml.SafeLoader, ) return yaml.safe_load(stream) finally: yaml.add_constructor( yaml.resolver.BaseResolver.DEFAULT_SEQUENCE_TAG, constructor_backup, Loader=yaml.SafeLoader, ) ```
95-martin-orion commented 2 weeks ago

Thanks for the fix!