Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for pathlib.Path to OmegaConf.load() and OmegaConf.save() #158

Merged
merged 4 commits into from
Feb 28, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions omegaconf/omegaconf.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import copy
import io
import os
import pathlib
import re
import sys
from contextlib import contextmanager
Expand Down Expand Up @@ -183,7 +184,7 @@ def create( # noqa F811
def load(file_: Union[str, IO[bytes]]) -> Union[DictConfig, ListConfig]:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the typing information on save and load to reflect this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

from ._utils import get_yaml_loader

if isinstance(file_, str):
if isinstance(file_, (str, pathlib.Path)):
with io.open(os.path.abspath(file_), "r", encoding="utf-8") as f:
obj = yaml.load(f, Loader=get_yaml_loader())
assert isinstance(obj, (list, dict, str))
Expand All @@ -204,7 +205,7 @@ def save(config: Container, f: Union[str, IO[str]], resolve: bool = False) -> No
:param resolve: True to save a resolved config (defaults to False)
"""
data = config.pretty(resolve=resolve)
if isinstance(f, str):
if isinstance(f, (str, pathlib.Path)):
with io.open(os.path.abspath(f), "w", encoding="utf-8") as file:
file.write(data)
elif hasattr(f, "write"):
Expand Down
21 changes: 21 additions & 0 deletions tests/test_serialization.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# -*- coding: utf-8 -*-
import io
import os
import pathlib
import tempfile
from typing import Any, Dict

Expand Down Expand Up @@ -37,6 +38,20 @@ def save_load_from_filename(conf: Container, resolve: bool, expected: Any) -> No
os.unlink(fp.name)


def save_load_from_pathlib_path(conf: Container, resolve: bool, expected: Any) -> None:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of duplicating the other function please parametrize it with pytest.mark.parametrize()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @omry.

I was able to implement the other suggestions, and while I would like to keep things DRY, I'm not quite sure how to parametrise using the original save_load_from_filename. Could you please point me in the right direction?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made an implementation. Hopefully this is reasonably elegant :)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good, and trickier than usual because you can't construct the object outside due to the temp file.
good job :).

if expected is None:
expected = conf
# note that delete=False here is a work around windows incompetence.
try:
with tempfile.NamedTemporaryFile(delete=False) as fp:
filepath = pathlib.Path(fp.name)
OmegaConf.save(conf, filepath, resolve=resolve)
c2 = OmegaConf.load(filepath)
assert c2 == expected
finally:
os.unlink(fp.name)


def test_load_from_invalid() -> None:
with pytest.raises(TypeError):
OmegaConf.load(3.1415) # type: ignore
Expand Down Expand Up @@ -64,6 +79,12 @@ def test_save_load__from_filename(
cfg = OmegaConf.create(input_)
save_load_from_filename(cfg, resolve, expected)

def test_save_load__from_pathlib_path(
self, input_: Dict[str, Any], resolve: bool, expected: Any
) -> None:
cfg = OmegaConf.create(input_)
save_load_from_pathlib_path(cfg, resolve, expected)


def test_save_illegal_type() -> None:
with pytest.raises(TypeError):
Expand Down