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
- nilearn>=0.9.0,<=0.10.0
- sqlalchemy>=1.4.27,<= 1.5.0
- pyyaml>=5.1.2,<7.0
- ruamel.yaml=0.17.*
- h5py=3.8.*
- seaborn=0.11.*
- 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
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>
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``
* ``nilearn>=0.9.0,<=0.10.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``
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
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
as pre-release to PyPI.

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -8,12 +8,18 @@ from pathlib import Path
from typing import Tuple
import pytest
import yaml
from click.testing import CliRunner
from ruamel.yaml import YAML
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
runner = CliRunner()
@ -33,19 +39,17 @@ def test_run_and_collect_commands(
# Get test config
infile = Path(__file__).parent / "data" / "gmd_mean.yaml"
# Read test config
with open(infile, mode="r") as f:
contents = yaml.safe_load(f)
contents = yaml.load(infile)
# Working directory
workdir = tmp_path / "workdir"
contents["workdir"] = str(workdir.absolute())
contents["workdir"] = str(workdir.resolve())
# Output directory
outdir = tmp_path / "outdir"
# Storage
contents["storage"]["uri"] = str(outdir.absolute())
contents["storage"]["uri"] = str(outdir.resolve())
# Write new test config
outfile = tmp_path / "in.yaml"
with open(outfile, mode="w") as f:
yaml.dump(contents, f)
yaml.dump(contents, stream=outfile)
# Run command arguments
run_args = [
str(outfile.absolute()),

View file

@ -11,7 +11,7 @@ from pathlib import Path
from typing import List, Tuple, Union
import pytest
import yaml
from ruamel.yaml import YAML
import junifer.testing.registry # noqa: F401
from junifer.api.functions import collect, queue, run
@ -19,6 +19,12 @@ from junifer.datagrabber.base import BaseDataGrabber
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
datagrabber = {
"kind": "OasisVBMTestingDatagrabber",
@ -231,6 +237,7 @@ def test_run_and_collect(tmp_path: Path) -> None:
def test_queue_correct_yaml_config(
tmp_path: Path,
monkeypatch: pytest.MonkeyPatch,
caplog: pytest.LogCaptureFixture,
) -> None:
"""Test proper YAML config generation for queueing.
@ -240,32 +247,37 @@ def test_queue_correct_yaml_config(
The path to the test directory.
monkeypatch : pytest.MonkeyPatch
The monkeypatch object.
caplog : pytest.LogCaptureFixture
The logcapturefixture object.
"""
with monkeypatch.context() as m:
m.chdir(tmp_path)
queue(
config={
"with": "junifer.testing.registry",
"workdir": str(Path(tmp_path).resolve()),
"datagrabber": datagrabber,
"markers": markers,
"storage": storage,
"env": {
"kind": "conda",
"name": "junifer",
with caplog.at_level(logging.INFO):
queue(
config={
"with": "junifer.testing.registry",
"workdir": str(tmp_path.resolve()),
"datagrabber": datagrabber,
"markers": markers,
"storage": {"kind": "SQLiteFeatureStorage"},
"env": {
"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(
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.unsafe_load(f)
yaml_config = yaml.load(generated_config_yaml_path)
# 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(
key in yaml_config.keys()

View file

@ -9,10 +9,19 @@ import re
from importlib.metadata import distribution
from typing import Dict
from ruamel.yaml import YAML
from .._version import __version__
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]:
"""Get junifer version information.
@ -71,16 +80,13 @@ def _get_dependency_information(long_: bool) -> Dict[str, str]:
# Get dependencies for junifer
dist = distribution("junifer")
# Compile regex pattern
re_pattern = re.compile("[a-z-]+")
re_pattern = re.compile("[a-z-_.]+")
for pkg_with_version in dist.requires: # type: ignore
# Perform regex search
matches = re.findall(pattern=re_pattern, string=pkg_with_version)
# Fix issue with PyYAML name registration
if matches[0] == "pyyaml":
key = "yaml"
else:
key = matches[0]
# Extract package name
key = matches[0]
if key in dependency_versions.keys():
# Check if pkg part of optional dependencies

View file

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

View file

@ -90,7 +90,9 @@ def get_versions() -> Dict:
"""
module_versions = {}
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
if name in ["_curses"]:
continue

View file

@ -46,7 +46,7 @@ dependencies = [
"nibabel>=3.2.0,<4.1",
"nilearn>=0.9.0,<=0.10.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'",
"h5py>=3.8.0,<3.9",
]