patch: config file issues

Co-authored-by: JoschD <joschua.dilly@gmail.com>
Co-authored-by: Felix Soubelet <felix.soubelet@protonmail.com>

CHANGELOG.md

@@ -1,5 +1,14 @@
 # `pylhc-submitter` Changelog
 
+## Version 1.0.1
+
+Version `1.0.1` is a patch release.
+
+- Fixed:
+    - Fixed an issue where the `config.ini` file created during submission would be saved in the `site-packages` instead of the specified `working_directory` if the submitter was installed and called as a module (`python -m pylhc_submitter.job_submitter ...`) ([pull/16](https://github.com/pylhc/submitter/pull/16)).
+    - Fixed an issue where `%` characters in a config file used for submission would cause the parameter parsing to crash ([pull/16](https://github.com/pylhc/submitter/pull/16)).
+    - Stopped the use of `OrderedDict` which would be written down to the `config.ini` file and prevent further use of said file ([pull/16](https://github.com/pylhc/submitter/pull/16)).
+
 ## Version 1.0.0
 
 First stable version of `pylhc_submitter`.

pylhc_submitter/__init__.py

@@ -10,7 +10,7 @@
 __title__ = "pylhc_submitter"
 __description__ = "pylhc-submitter contains scripts to simplify the creation and submission of jobs to HTCondor at CERN"
 __url__ = "https://github.com/pylhc/submitter"
-__version__ = "1.0.0"
+__version__ = "1.0.1"
 __author__ = "pylhc"
 __author_email__ = "pylhc@github.com"
 __license__ = "MIT"

pylhc_submitter/autosix.py

@@ -143,8 +143,6 @@
 from pylhc_submitter.job_submitter import (
     JOBSUMMARY_FILE,
     COLUMN_JOBID,
-    check_replace_dict,
-    keys_to_path,
 )
 from pylhc_submitter.sixdesk_tools.create_workspace import (
     create_job,
@@ -161,8 +159,18 @@
     sixdb_cmd,
     sixdb_load,
 )
-from pylhc_submitter.sixdesk_tools.utils import is_locked, check_mask, check_stage, StageSkip
-from pylhc_submitter.utils.iotools import PathOrStr, save_config
+from pylhc_submitter.sixdesk_tools.utils import (
+    is_locked,
+    check_mask,
+    check_stage,
+    StageSkip
+)
+from pylhc_submitter.utils.iotools import (
+    PathOrStr,
+    save_config,
+    make_replace_entries_iterable,
+    keys_to_path
+)
 
 LOG = logging.getLogger(__name__)
 
@@ -275,7 +283,7 @@ def main(opt):
     with open(opt.mask, "r") as mask_f:
         mask = mask_f.read()
     opt = _check_opts(mask, opt)
-    save_config(opt.working_directory, opt, __file__)
+    save_config(opt.working_directory, opt, "autosix")
 
     jobdf = _generate_jobs(opt.working_directory, opt.jobid_mask, **opt.replace_dict)
     for job_args in jobdf.iterrows():
@@ -481,7 +489,7 @@ def setup_and_run(jobname: str, basedir: Path, **kwargs):
 def _check_opts(mask_text, opt):
     opt = keys_to_path(opt, "mask", "working_directory", "executable")
     check_mask(mask_text, opt.replace_dict)
-    opt.replace_dict = check_replace_dict(opt.replace_dict)
+    opt.replace_dict = make_replace_entries_iterable(opt.replace_dict)
     return opt
 
 

pylhc_submitter/job_submitter.py

@@ -82,9 +82,6 @@
 import multiprocessing
 import subprocess
 import sys
-from collections import OrderedDict
-from collections.abc import Iterable
-from functools import partial
 from pathlib import Path
 
 import numpy as np
@@ -109,7 +106,7 @@
     JOBFLAVOURS,
 )
 from pylhc_submitter.utils.environment_tools import on_windows
-from pylhc_submitter.utils.iotools import PathOrStr, save_config
+from pylhc_submitter.utils.iotools import PathOrStr, save_config, make_replace_entries_iterable, keys_to_path
 from pylhc_submitter.utils.logging_tools import log_setup
 
 JOBSUMMARY_FILE = "Jobs.tfs"
@@ -278,7 +275,7 @@ def main(opt):
         LOG.info("Starting Job-submitter.")
 
     opt = _check_opts(opt)
-    save_config(opt.working_directory, opt, __file__)
+    save_config(opt.working_directory, opt, "job_submitter")
 
     job_df = _create_jobs(
         opt.working_directory,
@@ -526,7 +523,7 @@ def _check_opts(opt):
     check_percentage_signs_in_mask(mask)
 
     print_dict_tree(opt, name="Input parameter", print_fun=LOG.debug)
-    opt.replace_dict = check_replace_dict(opt.replace_dict)
+    opt.replace_dict = make_replace_entries_iterable(opt.replace_dict)
     return opt
 
 
@@ -544,25 +541,6 @@ def _print_stats(new_jobs, finished_jobs):
         LOG.info(job_name)
 
 
-# Other ------------------------------------------------------------------------
-
-
-def check_replace_dict(replace_dict: dict) -> OrderedDict:
-    """ Makes all entries in replace-dict iterable. """
-    for key, value in replace_dict.items():
-        if isinstance(value, str) or not isinstance(value, Iterable):
-            replace_dict[key] = [value]
-    return OrderedDict(replace_dict)  # for python 3.6
-
-
-def keys_to_path(dict_, *keys):
-    """ Convert all keys to Path, if they are not None. """
-    for key in keys:
-        value = dict_[key]
-        dict_[key] = None if value is None else Path(value)
-    return dict_
-
-
 # Script Mode ------------------------------------------------------------------
 
 

pylhc_submitter/utils/iotools.py

@@ -6,13 +6,17 @@
 """
 from pathlib import Path
 from datetime import datetime
+from typing import Iterable
 
 from generic_parser.entry_datatypes import get_instance_faker_meta
 from generic_parser.entrypoint_parser import save_options_to_config
 
 from pylhc_submitter.constants.general import TIME
 
 
+# Output -----------------------------------------------------------------------
+
+
 def save_config(output_dir: Path, opt: dict, script: str):
     """
     Quick wrapper for ``save_options_to_config``.
@@ -25,12 +29,24 @@ def save_config(output_dir: Path, opt: dict, script: str):
     """
     output_dir.mkdir(parents=True, exist_ok=True)
     opt = convert_paths_in_dict_to_strings(opt)
+    opt = escape_percentage_signs(opt)
     time = datetime.utcnow().strftime(TIME)
     save_options_to_config(output_dir / f"{script:s}_{time:s}.ini",
                            dict(sorted(opt.items()))
                            )
 
 
+def escape_percentage_signs(dict_: dict) -> dict:
+    """Escape all percentage signs.
+    They are used for interpolation in the config-parser."""
+    for key, value in dict_.items():
+        if isinstance(value, dict):
+            dict_[key] = escape_percentage_signs(value)
+        elif isinstance(value, str):
+            dict_[key] = value.replace("%(", "%%(")
+    return dict_
+
+
 def convert_paths_in_dict_to_strings(dict_: dict) -> dict:
     """Converts all Paths in the dict to strings, including those in iterables."""
     dict_ = dict_.copy()
@@ -53,6 +69,9 @@ def convert_paths_in_dict_to_strings(dict_: dict) -> dict:
     return dict_
 
 
+# Input ------------------------------------------------------------------------
+
+
 class PathOrStr(metaclass=get_instance_faker_meta(Path, str)):
     """A class that behaves like a Path when possible, otherwise like a string."""
     def __new__(cls, value):
@@ -62,3 +81,19 @@ def __new__(cls, value):
         if isinstance(value, str):
             value = value.strip("\'\"")  # behavior like dict-parser, IMPORTANT FOR EVERY STRING-FAKER
         return Path(value)
+
+
+def make_replace_entries_iterable(replace_dict: dict) -> dict:
+    """ Makes all entries in replace-dict iterable. """
+    for key, value in replace_dict.items():
+        if isinstance(value, str) or not isinstance(value, Iterable):
+            replace_dict[key] = [value]
+    return replace_dict
+
+
+def keys_to_path(dict_, *keys):
+    """ Convert all keys to Path, if they are not None. """
+    for key in keys:
+        value = dict_[key]
+        dict_[key] = None if value is None else Path(value)
+    return dict_

tests/unit/test_autosix.py

@@ -9,7 +9,8 @@
 from pylhc_submitter.autosix import _generate_jobs, setup_and_run
 from pylhc_submitter.constants.autosix import (
     get_masks_path, get_autosix_results_path, get_sixdeskenv_path,
-    get_sysenv_path, get_stagefile_path, ANGLE, Stage, get_mad6t_mask_path, get_mad6t1_mask_path
+    get_sysenv_path, get_stagefile_path, ANGLE, Stage,
+    get_mad6t_mask_path, get_mad6t1_mask_path
 )
 
 STAGE_NAMES = [s.name for s in Stage]

tests/unit/test_io.py

@@ -0,0 +1,79 @@
+from pathlib import Path
+
+import pytest
+from generic_parser import EntryPoint, EntryPointParameters
+from generic_parser.entry_datatypes import DictAsString
+
+from pylhc_submitter.utils.iotools import PathOrStr, save_config, keys_to_path, make_replace_entries_iterable
+
+
+def test_escape_percentages(tmp_path):
+    """Check if percentage-signs are escaped correctly in ini."""
+    opt = entrypoint(mask="%(PARAM)d+%(PARAM)d")
+    save_load_and_check(tmp_path, opt)
+
+
+@pytest.mark.parametrize('pathparam', ['test', Path('test')], ids=['str', 'Path'])
+def test_pathstr(tmp_path, pathparam):
+    """Test PathOrStr datatype."""
+    opt = entrypoint(pathstr=pathparam)
+    assert isinstance(opt.pathstr, pathparam.__class__)
+
+    opt = keys_to_path(opt, 'pathstr')
+    assert isinstance(opt.pathstr, Path)
+
+    save_load_and_check(tmp_path, opt)
+
+
+def test_replace_dict(tmp_path):
+    """Test conversion to make replace dict iterable."""
+    opt = entrypoint(replace_dict={"A": 1, "B": [100, 200], "C": "ITERME"})
+    assert isinstance(opt.replace_dict, dict)
+
+    new_dict = make_replace_entries_iterable(opt.replace_dict.copy())
+    assert new_dict["A"] == [opt.replace_dict["A"]]
+    assert new_dict["C"] == [opt.replace_dict["C"]]
+    assert new_dict["B"] == opt.replace_dict["B"]
+
+    opt.replace_dict = new_dict
+    save_load_and_check(tmp_path, opt)
+
+
+def test_multiple_entries(tmp_path):
+    """Test config file for multiple entries from above."""
+    opt = entrypoint(replace_dict={"A": 1, "B": [100, 200], "C": "ITERME"},
+                     pathstr=Path("test"),
+                     mask="%(PARAM)d+%(PARAM)d")
+
+    save_load_and_check(tmp_path, opt)
+
+
+# Helper -----------------------------------------------------------------------
+
+
+def save_load_and_check(path, opt):
+    """Save opt, load as config file and runs basic tests."""
+    save_config(path, opt, "test")
+    cfg_file = next(path.glob("*.ini"))
+    assert cfg_file.exists() and cfg_file.is_file()
+
+    opt_load = entrypoint(entry_cfg=cfg_file)
+    for key in opt:
+        assert opt[key] == opt_load[key]
+
+
+def entrypoint(**kwargs):
+    """Creates an entrypoint and parses kwargs."""
+    params = EntryPointParameters(
+        pathstr=dict(
+            type=PathOrStr,
+        ),
+        replace_dict=dict(
+            type=DictAsString,
+        ),
+        mask=dict(
+            type=str,
+        )
+    )
+    entry = EntryPoint(params)
+    return entry.parse(**kwargs)[0]

tests/unit/test_job_submitter.py

@@ -1,4 +1,3 @@
-import sys
 from pathlib import Path
 
 import pytest