Merge pull request #1724 from rfbgo/plural_type

abstract key normlization and support type plurals

Author: douglasjacobsen

lib/ramble/ramble/cmd/common/info.py

@@ -484,7 +484,7 @@ def print_info(args):
     if spec.object_type:
         object_type = spec.object_type
     else:
-        object_type = ramble.repository.ObjectTypes[args.type]
+        object_type = ramble.repository.simplify_object_type(args.type)
 
     obj = ramble.repository.get(spec, object_type=object_type)
 

lib/ramble/ramble/cmd/common/list.py

@@ -162,7 +162,7 @@ def perform_list(args):
     # retrieve the formatter to use from args
     formatter = formatters[args.format]
 
-    object_type = ramble.repository.ObjectTypes[args.type]
+    object_type = ramble.repository.simplify_object_type(args.type)
 
     sorted_objects = object_utils.filter_by_name(args.filter, args.search_description, object_type)
 

lib/ramble/ramble/cmd/edit.py

@@ -41,12 +41,7 @@ def normalize_type_name(type_name):
     if norm_type in extra_type_aliases:
         return extra_type_aliases[norm_type]
 
-    # Map object types using repository's get_object_type_map()
-    type_map = ramble.repository.get_object_type_map()
-    if norm_type in type_map:
-        return type_map[norm_type].name
-
-    return type_name
+    return ramble.repository.simplify_object_type(type_name).name
 
 
 def find_all_matches(name, repo_path=None, namespace=None, obj_type=None):
@@ -162,11 +157,6 @@ def edit(parser, args):
     # Normalize input type if specified
     if args.type:
         args.type = normalize_type_name(args.type)
-        extra_types = ["test", "command", "docs", "module"]
-        allowed_types = ramble.repository.OBJECT_NAMES + extra_types
-        if args.type not in allowed_types:
-            # Trigger KeyError like the original code did
-            _ = ramble.repository.ObjectTypes[args.type]
 
     if name:
         matches = find_all_matches(name, args.repo, args.namespace, args.type)

lib/ramble/ramble/cmd/repo.py

@@ -360,7 +360,7 @@ def repo(parser, args):
         "rm": repo_remove,
     }
 
-    if args.type != "any" and args.type not in ramble.repository.OBJECT_NAMES:
-        logger.die(f"Repository type '{args.type}' is not valid.")
+    if args.type != "any":
+        args.type = ramble.repository.simplify_object_type(args.type).name
 
     action[args.repo_command](args)

lib/ramble/ramble/repository.py

@@ -101,6 +101,7 @@
         "config_section": "package_manager_repos",
         "accepted_configs": ["package_manager_repo.yaml", unified_config],
         "singular": "package manager",
+        "aliases": ["pkg", "package"],
     },
     ObjectTypes.workflow_managers: {
         "file_name": "workflow_manager.py",
@@ -109,6 +110,7 @@
         "config_section": "workflow_manager_repos",
         "accepted_configs": ["workflow_manager_repo.yaml", unified_config],
         "singular": "workflow manager",
+        "aliases": ["workflow"],
     },
     ObjectTypes.systems: {
         "file_name": "system.py",
@@ -133,6 +135,7 @@
         "config_section": "base_class_repos",
         "accepted_configs": ["base_class_repo.yaml", unified_config],
         "singular": "base class",
+        "aliases": ["base"],
     },
     ObjectTypes.base_applications: {
         "file_name": "base_application.py",
@@ -157,6 +160,7 @@
         "config_section": "base_package_manager_repos",
         "accepted_configs": ["base_package_manager_repo.yaml", unified_config],
         "singular": "base package manager",
+        "aliases": ["base_pkg"],
     },
     ObjectTypes.base_workflow_managers: {
         "file_name": "base_workflow_manager.py",
@@ -165,6 +169,7 @@
         "config_section": "base_workflow_manager_repos",
         "accepted_configs": ["base_workflow_manager_repo.yaml", unified_config],
         "singular": "base workflow manager",
+        "aliases": ["base_workflow"],
     },
     ObjectTypes.base_systems: {
         "file_name": "base_system.py",
@@ -205,26 +210,64 @@
 }
 
 
-@functools.lru_cache(maxsize=1)
+def _normalize_type_key(key):
+    return str(key).lower().replace("-", "_").replace(" ", "_")
+
+
+_TYPE_ALIASES = {}
+
+for _obj in ObjectTypes:
+    _tdef = type_definitions.get(_obj, {})
+    _candidates = [
+        _obj.name,
+        _tdef.get("singular"),
+        _tdef.get("abbrev"),
+        _tdef.get("dir_name"),
+        *_tdef.get("aliases", []),
+    ]
+    for _val in _candidates:
+        if isinstance(_val, str):
+            for _v in (_val, f"{_val}s"):
+                _norm = _normalize_type_key(_v)
+                _TYPE_ALIASES[_norm] = _obj
+                _TYPE_ALIASES[_norm.replace("_", "-")] = _obj
+
+
 def get_object_type_map():
     """Returns a mapping from string representations of object types (singular,
     plural, abbrev, hyphens/underscores) to their corresponding ObjectType enum."""
-    mapping = {}
-    for obj_type, type_def in type_definitions.items():
-        candidates = set()
-        for key in ("abbrev", "dir_name", "singular"):
-            val = type_def.get(key)
-            if val:
-                val = val.replace(" ", "_")
-                candidates.update([val, val.replace("_", "-"), val.replace("-", "_")])
+    return _TYPE_ALIASES
+
 
-        for cand in candidates:
-            mapping[cand] = obj_type
-    return mapping
+def simplify_object_type(type_name):
+    """Convert a type string or ObjectTypes member to an ObjectTypes enum member.
+
+    Args:
+        type_name (ObjectTypes | str): Object type to simplify / normalize.
+
+    Returns:
+        (ObjectTypes): The matching ObjectTypes enum member.
+
+    Raises:
+        UnknownObjectTypeError: If type_name does not match any valid object type.
+    """
+    if isinstance(type_name, ObjectTypes):
+        return type_name
+
+    if isinstance(type_name, str):
+        key = _normalize_type_key(type_name)
+        if key in _TYPE_ALIASES:
+            return _TYPE_ALIASES[key]
+
+    raise UnknownObjectTypeError(type_name)
+
+
+get_object_type = simplify_object_type
 
 
 def _gen_path(repo_dirs=None, obj_type=default_type):
     """Create a RepoPath for a specific object, add it to sys.meta_path, and return it."""
+    obj_type = simplify_object_type(obj_type)
     section_name = type_definitions[obj_type]["config_section"]
     singular_name = type_definitions[obj_type]["singular"]
     repo_dirs = repo_dirs or ramble.config.get(section_name)
@@ -259,6 +302,7 @@ def list_object_files(obj_inst, object_type):
     This is currently used by `ramble deployment` to copy relevant files
     to create a self-contained repo.
     """
+    object_type = simplify_object_type(object_type)
     type_def = type_definitions[object_type]
     base_type = ObjectTypes[f"base_{type_def['dir_name']}"]
     base_type_def = type_definitions[base_type]
@@ -299,11 +343,13 @@ def list_object_files(obj_inst, object_type):
 
 def all_object_names(object_type=default_type):
     """Convenience wrapper around ``ramble.repository.all_object_names()``."""
+    object_type = simplify_object_type(object_type)
     return paths[object_type].all_object_names()
 
 
 def get(spec, object_type=default_type):
     """Convenience wrapper around ``ramble.repository.get()``."""
+    object_type = simplify_object_type(object_type)
     return paths[object_type].get(spec)
 
 
@@ -314,6 +360,7 @@ def get_base_class(spec):
 
 def get_obj_class(spec, object_type=default_type):
     """Convenience wrapper around ``ramble.repository.get_obj_class()``."""
+    object_type = simplify_object_type(object_type)
     return paths[object_type].get_obj_class(spec)
 
 
@@ -324,6 +371,7 @@ def set_path(repo, object_type=default_type):
     ``sys.meta_path`` if it is a ``Repo`` or ``RepoPath``.
     """
     global paths  # noqa: F824
+    object_type = simplify_object_type(object_type)
     paths[object_type] = repo
 
     # make the new repo_path an importer if needed
@@ -345,6 +393,7 @@ def use_repositories(*paths_and_repos, object_type=default_type):
         RepoPath: Corresponding RepoPath object
     """
     global paths  # noqa: F824
+    object_type = simplify_object_type(object_type)
 
     # Construct a temporary RepoPath object from
     temporary_repositories = RepoPath(*paths_and_repos, object_type=object_type)
@@ -1572,6 +1621,18 @@ class RepoError(ramble.error.RambleError):
     """Superclass for repository-related errors."""
 
 
+class UnknownObjectTypeError(RepoError):
+    """Raised when an unknown or invalid object type is specified."""
+
+    def __init__(self, type_name):
+        valid_types = ", ".join(OBJECT_NAMES)
+        super().__init__(
+            f"Unknown object type '{type_name}'.",
+            f"Allowed types are: {valid_types} "
+            "(singular, plural, and abbreviation forms are accepted).",
+        )
+
+
 class NoRepoConfiguredError(RepoError):
     """Raised when there are no repositories configured."""
 

lib/ramble/ramble/test/cmd/edit.py

@@ -8,6 +8,7 @@
 
 import pytest
 
+import ramble.repository
 from ramble.main import RambleCommand
 
 edit = RambleCommand("edit")
@@ -118,7 +119,7 @@ def test_edit_singular_type_with_spaces_hyphens(mock_editor):
 
 
 def test_edit_unknown_type():
-    with pytest.raises(KeyError):
+    with pytest.raises(ramble.repository.UnknownObjectTypeError):
         edit("-t", "unknown_type", "spack")
 
 
@@ -174,8 +175,8 @@ def test_normalize_type_name():
     assert normalize_type_name("TEST") == "test"
     assert normalize_type_name("Command") == "command"
     assert normalize_type_name("APP") == "applications"
-    assert normalize_type_name("MODIFIER") == "modifiers"
-    assert normalize_type_name("unknown_type") == "unknown_type"
+    with pytest.raises(ramble.repository.UnknownObjectTypeError):
+        normalize_type_name("unknown_type")
 
 
 def test_edit_abbreviated_type(mock_modifiers, mock_editor):
@@ -272,6 +273,10 @@ def test_edit_no_name_with_type_editor(mock_editor):
     assert len(mock_editor) == 2
     assert "var/ramble/repos/builtin" in mock_editor[1]
 
+    edit("-t", "application")
+    assert len(mock_editor) == 3
+    assert "var/ramble/repos/builtin" in mock_editor[2]
+
 
 def test_edit_no_name_with_custom_type_repo_editor(mock_editor):
     edit("-t", "applications", "--repo", "/non-existent-path")

lib/ramble/ramble/test/cmd/info.py

@@ -110,9 +110,14 @@ def test_mock_spack_info_software(mock_applications, app_query):
     "info_query",
     [
         ["--type", "modifiers", "apptainer"],
+        ["--type", "modifier", "apptainer"],
         ["--type", "modifiers", "apptainer", "-vv"],
         ["--type", "package_managers", "spack"],
+        ["--type", "package_manager", "spack"],
+        ["--type", "package-manager", "spack"],
         ["--type", "workflow_managers", "slurm"],
+        ["--type", "workflow_manager", "slurm"],
+        ["--type", "workflow-manager", "slurm"],
     ],
 )
 def test_non_app_object_info_common_fields(info_query):

lib/ramble/ramble/test/cmd/list.py

@@ -75,6 +75,30 @@ def test_list_base_html():
     assert '<div class="section" id="hpl">' in output
 
 
+@pytest.mark.parametrize(
+    "type_arg",
+    [
+        "applications",
+        "application",
+        "app",
+        "base_applications",
+        "base_application",
+        "base-application",
+        "modifiers",
+        "modifier",
+        "package_managers",
+        "package_manager",
+        "package-manager",
+        "workflow_managers",
+        "workflow_manager",
+    ],
+)
+def test_list_types(type_arg):
+    output = list("--type", type_arg)
+    assert output is not None
+    assert len(output) > 0
+
+
 def test_list_update(tmpdir):
     update_file = tmpdir.join("output")
 

lib/ramble/ramble/test/cmd/repo.py

@@ -181,3 +181,19 @@ def test_add_repo_missing_config_file(mutable_config, tmpdir):
 
     expected_error_message = f"No valid config file found in '{repo_path}'"
     assert expected_error_message in str(e.value)
+
+
+@pytest.mark.parametrize("type_arg", ["application", "app"])
+def test_singular_type_repo_commands(mutable_config, tmpdir, type_arg):
+    repo_path = str(tmpdir.join(f"test_repo_{type_arg}"))
+    repo("create", repo_path, f"mockrepo_{type_arg}", "-t", type_arg)
+    assert os.path.exists(os.path.join(repo_path, "application_repo.yaml"))
+    assert os.path.exists(os.path.join(repo_path, "applications"))
+
+    repo("add", "-t", type_arg, "--scope=site", repo_path)
+    output = repo("list", "-t", type_arg, "--scope=site", output=str)
+    assert f"mockrepo_{type_arg}" in output
+
+    repo("remove", "-t", type_arg, "--scope=site", repo_path)
+    output = repo("list", "-t", type_arg, "--scope=site", output=str)
+    assert f"mockrepo_{type_arg}" not in output