Use ruamel.yaml in place of pyyaml #223

Merged
synchon merged 17 commits from update/ruamel.yaml into main 2023-05-09 11:07:29 +00:00
17 changed files with 73 additions and 50 deletions

View file

@ -10,7 +10,7 @@ dependencies:
- nibabel>=3.2.0,<4.1 - nibabel>=3.2.0,<4.1
- nilearn>=0.9.0,<=0.10.0 - nilearn>=0.9.0,<=0.10.0
- sqlalchemy>=1.4.27,<= 1.5.0 - sqlalchemy>=1.4.27,<= 1.5.0
- pyyaml>=5.1.2,<7.0 - ruamel.yaml=0.17.*
- h5py=3.8.* - h5py=3.8.*
- seaborn=0.11.* - seaborn=0.11.*
- Sphinx=5.3.* - Sphinx=5.3.*

View file

@ -0,0 +1 @@
Enable YAML 1.2 support and allow multiline strings in YAML which would not work earlier by `Synchon Mandal`_

View file

@ -0,0 +1 @@
Use ``ruamel.yaml`` instead of ``pyyaml`` as YAML I/O library by `Synchon Mandal`_

View file

@ -23,7 +23,7 @@ The following steps are specific to VSCode and you can choose to go with it:
.. code-block:: bash .. code-block:: bash
conda env create -n <your-environment-name> -f conda-env.yml python=3.10 conda env create -n <your-environment-name> -f conda-env.yml
LeSasse commented 2023-05-08 10:40:31 +00:00 (Migrated from github.com)

What is the reason for not having to specify the python version in the command anymore?

What is the reason for not having to specify the python version in the command anymore?
synchon commented 2023-05-08 13:25:45 +00:00 (Migrated from github.com)

As we have that constraint (python=3.10) in the conda-env.yml and also this syntax is incorrect from what I remember.

As we have that constraint (`python=3.10`) in the `conda-env.yml` and also this syntax is incorrect from what I remember.
conda activate <your-environment-name> conda activate <your-environment-name>
The ``conda-env.yml`` can be found at the root of the repository. The ``conda-env.yml`` can be found at the root of the repository.

View file

@ -16,7 +16,7 @@ junifer is compatible with `Python`_ >= 3.8 and requires the following packages:
* ``nibabel>=3.2.0,<4.1`` * ``nibabel>=3.2.0,<4.1``
* ``nilearn>=0.9.0,<=0.10.0`` * ``nilearn>=0.9.0,<=0.10.0``
* ``sqlalchemy>=1.4.27,<= 1.5.0`` * ``sqlalchemy>=1.4.27,<= 1.5.0``
* ``pyyaml>=5.1.2,<7.0`` * ``ruamel.yaml>=0.17,<0.18``
* ``h5py>=3.8.0,<3.9`` * ``h5py>=3.8.0,<3.9``
Depending on the installation method, these packages might be installed Depending on the installation method, these packages might be installed

View file

@ -16,7 +16,7 @@ This plugin reads the latest tagged version from git and automatically
increments the *MICRO* segment and appends *devN*. This is considered a increments the *MICRO* segment and appends *devN*. This is considered a
pre-release. pre-release.
The CI scripts will publish every tag with the format *v.X.Y.Z* to Pypi as The CI scripts will publish every tag with the format *v.X.Y.Z* to PyPI as
version "X.Y.Z". Additionally, for every push to main, it will be published version "X.Y.Z". Additionally, for every push to main, it will be published
as pre-release to PyPI. as pre-release to PyPI.

View file

@ -11,7 +11,6 @@ from pathlib import Path
from typing import Dict, List, Union from typing import Dict, List, Union
import click import click
import yaml
from ..utils.logging import ( from ..utils.logging import (
configure_logging, configure_logging,
@ -29,6 +28,7 @@ from .utils import (
_get_junifer_version, _get_junifer_version,
_get_python_information, _get_python_information,
_get_system_information, _get_system_information,
yaml,
) )
@ -275,7 +275,7 @@ def wtf(long_: bool) -> None:
"system": _get_system_information(), "system": _get_system_information(),
"environment": _get_environment_information(long_=long_), "environment": _get_environment_information(long_=long_),
} }
click.echo(yaml.dump(report, sort_keys=False)) click.echo(yaml.dump(report, stream=sys.stdout))
@cli.command() @cli.command()

View file

@ -4,6 +4,7 @@
# Leonard Sasse <l.sasse@fz-juelich.de> # Leonard Sasse <l.sasse@fz-juelich.de>
# Synchon Mandal <s.mandal@fz-juelich.de> # Synchon Mandal <s.mandal@fz-juelich.de>
# License: AGPL # License: AGPL
from typing import Type from typing import Type
from ..pipeline.registry import register from ..pipeline.registry import register

View file

@ -12,8 +12,6 @@ import typing
from pathlib import Path from pathlib import Path
from typing import Dict, List, Optional, Tuple, Union from typing import Dict, List, Optional, Tuple, Union
import yaml
from ..datagrabber.base import BaseDataGrabber from ..datagrabber.base import BaseDataGrabber
from ..markers.base import BaseMarker from ..markers.base import BaseMarker
from ..markers.collection import MarkerCollection from ..markers.collection import MarkerCollection
@ -22,6 +20,7 @@ from ..preprocess.base import BasePreprocessor
from ..storage.base import BaseFeatureStorage from ..storage.base import BaseFeatureStorage
from ..utils import logger, raise_error from ..utils import logger, raise_error
from ..utils.fs import make_executable from ..utils.fs import make_executable
from .utils import yaml
def _get_datagrabber(datagrabber_config: Dict) -> BaseDataGrabber: def _get_datagrabber(datagrabber_config: Dict) -> BaseDataGrabber:
@ -270,8 +269,7 @@ def queue(
yaml_config = jobdir / "config.yaml" yaml_config = jobdir / "config.yaml"
logger.info(f"Writing YAML config to {str(yaml_config.absolute())}") logger.info(f"Writing YAML config to {str(yaml_config.absolute())}")
with open(yaml_config, "w") as f: yaml.dump(config, stream=yaml_config)
f.write(yaml.dump(config))
# Get list of elements # Get list of elements
if elements is None: if elements is None:

View file

@ -10,9 +10,8 @@ import sys
from pathlib import Path from pathlib import Path
from typing import Dict, Union from typing import Dict, Union
import yaml
from ..utils.logging import logger, raise_error from ..utils.logging import logger, raise_error
from .utils import yaml
def parse_yaml(filepath: Union[str, Path]) -> Dict: def parse_yaml(filepath: Union[str, Path]) -> Dict:
@ -38,8 +37,7 @@ def parse_yaml(filepath: Union[str, Path]) -> Dict:
if not filepath.exists(): if not filepath.exists():
raise_error(f"File does not exist: {str(filepath.absolute())}") raise_error(f"File does not exist: {str(filepath.absolute())}")
# Filepath reading # Filepath reading
with open(filepath, "r") as f: contents = yaml.load(filepath)
contents = yaml.safe_load(f)
if "elements" in contents: if "elements" in contents:
if contents["elements"] is None: if contents["elements"] is None:
raise_error( raise_error(

View file

@ -40,7 +40,7 @@ def test_get_dependency_information_short() -> None:
"nibabel", "nibabel",
"nilearn", "nilearn",
"sqlalchemy", "sqlalchemy",
"yaml", "ruamel.yaml",
] ]
@ -58,7 +58,7 @@ def test_get_dependency_information_long() -> None:
"nibabel", "nibabel",
"nilearn", "nilearn",
"sqlalchemy", "sqlalchemy",
"yaml", "ruamel.yaml",
]: ]:
assert key in dependency_information_keys assert key in dependency_information_keys

View file

@ -8,12 +8,18 @@ from pathlib import Path
from typing import Tuple from typing import Tuple
import pytest import pytest
import yaml
from click.testing import CliRunner from click.testing import CliRunner
from ruamel.yaml import YAML
from junifer.api.cli import collect, run, selftest, wtf from junifer.api.cli import collect, run, selftest, wtf
# Configure YAML class
yaml = YAML()
yaml.default_flow_style = False
yaml.allow_unicode = True
yaml.indent(mapping=2, sequence=4, offset=2)
# Create click test runner # Create click test runner
runner = CliRunner() runner = CliRunner()
@ -33,19 +39,17 @@ def test_run_and_collect_commands(
# Get test config # Get test config
infile = Path(__file__).parent / "data" / "gmd_mean.yaml" infile = Path(__file__).parent / "data" / "gmd_mean.yaml"
# Read test config # Read test config
with open(infile, mode="r") as f: contents = yaml.load(infile)
contents = yaml.safe_load(f)
# Working directory # Working directory
workdir = tmp_path / "workdir" workdir = tmp_path / "workdir"
contents["workdir"] = str(workdir.absolute()) contents["workdir"] = str(workdir.resolve())
# Output directory # Output directory
outdir = tmp_path / "outdir" outdir = tmp_path / "outdir"
# Storage # Storage
contents["storage"]["uri"] = str(outdir.absolute()) contents["storage"]["uri"] = str(outdir.resolve())
# Write new test config # Write new test config
outfile = tmp_path / "in.yaml" outfile = tmp_path / "in.yaml"
with open(outfile, mode="w") as f: yaml.dump(contents, stream=outfile)
yaml.dump(contents, f)
# Run command arguments # Run command arguments
run_args = [ run_args = [
str(outfile.absolute()), str(outfile.absolute()),

View file

@ -11,7 +11,7 @@ from pathlib import Path
from typing import List, Tuple, Union from typing import List, Tuple, Union
import pytest import pytest
import yaml from ruamel.yaml import YAML
import junifer.testing.registry # noqa: F401 import junifer.testing.registry # noqa: F401
from junifer.api.functions import collect, queue, run from junifer.api.functions import collect, queue, run
@ -19,6 +19,12 @@ from junifer.datagrabber.base import BaseDataGrabber
from junifer.pipeline.registry import build from junifer.pipeline.registry import build
# Configure YAML class
yaml = YAML()
yaml.default_flow_style = False
yaml.allow_unicode = True
yaml.indent(mapping=2, sequence=4, offset=2)
# Define datagrabber # Define datagrabber
datagrabber = { datagrabber = {
"kind": "OasisVBMTestingDatagrabber", "kind": "OasisVBMTestingDatagrabber",
@ -231,6 +237,7 @@ def test_run_and_collect(tmp_path: Path) -> None:
def test_queue_correct_yaml_config( def test_queue_correct_yaml_config(
tmp_path: Path, tmp_path: Path,
monkeypatch: pytest.MonkeyPatch, monkeypatch: pytest.MonkeyPatch,
caplog: pytest.LogCaptureFixture,
) -> None: ) -> None:
"""Test proper YAML config generation for queueing. """Test proper YAML config generation for queueing.
@ -240,32 +247,37 @@ def test_queue_correct_yaml_config(
The path to the test directory. The path to the test directory.
monkeypatch : pytest.MonkeyPatch monkeypatch : pytest.MonkeyPatch
The monkeypatch object. The monkeypatch object.
caplog : pytest.LogCaptureFixture
The logcapturefixture object.
""" """
with monkeypatch.context() as m: with monkeypatch.context() as m:
m.chdir(tmp_path) m.chdir(tmp_path)
queue( with caplog.at_level(logging.INFO):
config={ queue(
"with": "junifer.testing.registry", config={
"workdir": str(Path(tmp_path).resolve()), "with": "junifer.testing.registry",
"datagrabber": datagrabber, "workdir": str(tmp_path.resolve()),
"markers": markers, "datagrabber": datagrabber,
"storage": storage, "markers": markers,
"env": { "storage": {"kind": "SQLiteFeatureStorage"},
"kind": "conda", "env": {
"name": "junifer", "kind": "conda",
"name": "junifer",
},
"mem": "8G",
}, },
"mem": "8G", kind="HTCondor",
}, jobname="yaml_config_gen_check",
kind="HTCondor", )
jobname="yaml_config_gen_check", assert "Creating job in" in caplog.text
) assert "Writing YAML config to" in caplog.text
assert "Queue done" in caplog.text
generated_config_yaml_path = Path( generated_config_yaml_path = Path(
tmp_path / "junifer_jobs" / "yaml_config_gen_check" / "config.yaml" tmp_path / "junifer_jobs" / "yaml_config_gen_check" / "config.yaml"
) )
LeSasse commented 2023-05-08 10:45:47 +00:00 (Migrated from github.com)

Should you use CamelCase in the docstring here, similar to the DataGrabber issue?

Should you use CamelCase in the docstring here, similar to the DataGrabber issue?
synchon commented 2023-05-08 13:30:43 +00:00 (Migrated from github.com)

Resolving this as I'll take care of this in a separate PR.

Resolving this as I'll take care of this in a separate PR.
with open(generated_config_yaml_path, "r") as f: yaml_config = yaml.load(generated_config_yaml_path)
yaml_config = yaml.unsafe_load(f)
# Check for correct YAML config generation # Check for correct YAML config generation
LeSasse commented 2023-05-08 10:45:36 +00:00 (Migrated from github.com)

Should you use CamelCase in the docstring here, similar to the DataGrabber issue?

Should you use CamelCase in the docstring here, similar to the DataGrabber issue?
synchon commented 2023-05-08 11:43:55 +00:00 (Migrated from github.com)

Yeah I was debating that and I followed what was there but fair point. I'll take care of that in a separate PR for all test functions.

Yeah I was debating that and I followed what was there but fair point. I'll take care of that in a separate PR for all test functions.
LeSasse commented 2023-05-08 12:56:17 +00:00 (Migrated from github.com)

ok makes sense, fine by me!

ok makes sense, fine by me!
assert all( assert all(
key in yaml_config.keys() key in yaml_config.keys()

View file

@ -9,10 +9,19 @@ import re
from importlib.metadata import distribution from importlib.metadata import distribution
from typing import Dict from typing import Dict
from ruamel.yaml import YAML
from .._version import __version__ from .._version import __version__
from ..utils.logging import get_versions from ..utils.logging import get_versions
# Configure YAML class once for further use
yaml = YAML()
yaml.default_flow_style = False
yaml.allow_unicode = True
yaml.indent(mapping=2, sequence=4, offset=2)
LeSasse commented 2023-05-08 10:47:42 +00:00 (Migrated from github.com)

So this is a global object that you import elsewhere to load files?

So this is a global object that you import elsewhere to load files?
synchon commented 2023-05-08 11:44:59 +00:00 (Migrated from github.com)

Yeah cause now you gotta have an instance of YAML class so doesn't make sense to create multiple objects if you have the same thing everywhere.

Yeah cause now you gotta have an instance of `YAML` class so doesn't make sense to create multiple objects if you have the same thing everywhere.
def _get_junifer_version() -> Dict[str, str]: def _get_junifer_version() -> Dict[str, str]:
"""Get junifer version information. """Get junifer version information.
@ -71,16 +80,13 @@ def _get_dependency_information(long_: bool) -> Dict[str, str]:
# Get dependencies for junifer # Get dependencies for junifer
dist = distribution("junifer") dist = distribution("junifer")
# Compile regex pattern # Compile regex pattern
re_pattern = re.compile("[a-z-]+") re_pattern = re.compile("[a-z-_.]+")
for pkg_with_version in dist.requires: # type: ignore for pkg_with_version in dist.requires: # type: ignore
# Perform regex search # Perform regex search
matches = re.findall(pattern=re_pattern, string=pkg_with_version) matches = re.findall(pattern=re_pattern, string=pkg_with_version)
# Fix issue with PyYAML name registration # Extract package name
if matches[0] == "pyyaml": key = matches[0]
key = "yaml"
else:
key = matches[0]
if key in dependency_versions.keys(): if key in dependency_versions.keys():
# Check if pkg part of optional dependencies # Check if pkg part of optional dependencies

View file

@ -29,7 +29,7 @@ from junifer.storage.utils import (
("nibabel", "4.1"), ("nibabel", "4.1"),
("nilearn", "0.10.0"), ("nilearn", "0.10.0"),
("sqlalchemy", "1.5.0"), ("sqlalchemy", "1.5.0"),
("pyyaml", "7.0"), ("ruamel.yaml", "0.18.0"),
], ],
) )
def test_get_dependency_version(dependency: str, max_version: str) -> None: def test_get_dependency_version(dependency: str, max_version: str) -> None:

View file

@ -90,7 +90,9 @@ def get_versions() -> Dict:
""" """
module_versions = {} module_versions = {}
for name, module in sys.modules.items(): for name, module in sys.modules.items():
if "." in name: # Bypassing sub-modules of packages and
# allowing ruamel.yaml
if "." in name and name != "ruamel.yaml":
continue continue
if name in ["_curses"]: if name in ["_curses"]:
continue continue

View file

@ -46,7 +46,7 @@ dependencies = [
"nibabel>=3.2.0,<4.1", "nibabel>=3.2.0,<4.1",
"nilearn>=0.9.0,<=0.10.0", "nilearn>=0.9.0,<=0.10.0",
"sqlalchemy>=1.4.27,<= 1.5.0", "sqlalchemy>=1.4.27,<= 1.5.0",
"pyyaml>=5.1.2,<7.0", "ruamel.yaml>=0.17,<0.18",
"importlib_metadata; python_version < '3.10'", "importlib_metadata; python_version < '3.10'",
"h5py>=3.8.0,<3.9", "h5py>=3.8.0,<3.9",
] ]