diff --git a/README.md b/README.md index 4381972..4f5f0b7 100644 --- a/README.md +++ b/README.md @@ -117,6 +117,48 @@ print(settings.Canvas.canvas_scale) # Access settings from external libraries (if installed) print(settings.Reader.preferred_reader) # From ndevio print(settings.Export.compression_level) # From ndevio + +# Modify and save settings +settings.Canvas.canvas_scale = 2.0 +settings.save() # Persists across sessions + +# Reset to defaults +settings.reset_to_default("canvas_scale") # Reset single setting +settings.reset_to_default(group="Canvas") # Reset entire group +settings.reset_to_default() # Reset all settings +``` + +## Performance Note: npe1 Plugin Compatibility + +If you have many legacy npe1 plugins installed (e.g., `napari-assistant`, `napari-segment-blobs-and-things-with-membranes`, `napari-simpleitk-image-processing`), you may experience slow widget loading times (10+ seconds) the first time you open the settings plugin widget in a napari session. This is a known issue in napari's npe1 compatibility layer, not specific to ndev-settings. The npe1 adapter iterates through all plugin widgets and performs expensive metadata lookups for each legacy plugin. + +**Workaround**: If you don't need npe1 runtime behavior plugins, you can disable the adapter in napari: + +1. Go to `File` -> `Preferences` -> `Plugins` +2. Uncheck "Use npe2 adapter" +3. Restart napari + +This dramatically improves widget loading times since only pure npe2 plugins are discovered. + +## How Settings Persistence Works + +Settings are automatically cached to improve startup performance: + +1. **First load**: Settings are discovered from all installed packages via entry points, merged together, and saved to a user config file +2. **Subsequent loads**: Settings are loaded directly from the cached file (much faster) +3. **Package changes**: When packages are installed/removed, settings are re-discovered and merged while preserving your customizations + +**Storage location**: Settings are stored in a platform-appropriate config directory: + +- **Windows**: `%LOCALAPPDATA%\ndev-settings\settings.yaml` +- **macOS**: `~/Library/Application Support/ndev-settings/settings.yaml` +- **Linux**: `~/.config/ndev-settings/settings.yaml` + +**Clearing the cache**: To force re-discovery of settings (e.g., after manual edits to package YAML files): + +```python +from ndev_settings import clear_settings +clear_settings() # Deletes cached settings, next load will re-discover ``` ## Pre-commit hook diff --git a/pyproject.toml b/pyproject.toml index fde03cb..b0d1d65 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,6 +29,7 @@ requires-python = ">=3.10" # or any other Qt bindings directly (e.g. PyQt5, PySide2). # See best practices: https://napari.org/stable/plugins/building_a_plugin/best_practices.html dependencies = [ + "appdirs", "magicgui", "magic-class", "pyyaml", @@ -37,15 +38,16 @@ dependencies = [ [project.optional-dependencies] # Allow easily installation with the full, default napari installation # (including Qt backend) using ndev-settings[all]. -all = ["napari[all]"] +all = ["napari[pyqt6]"] [dependency-groups] -testing = [ +dev = [ "tox-uv", "pytest", # https://docs.pytest.org/en/latest/contents.html "pytest-cov", # https://pytest-cov.readthedocs.io/en/latest/ "pytest-qt", # https://pytest-qt.readthedocs.io/en/latest/ "pyqt6", + "napari", ] [project.entry-points."napari.manifest"] diff --git a/src/ndev_settings/__init__.py b/src/ndev_settings/__init__.py index 30761d6..eea74d0 100644 --- a/src/ndev_settings/__init__.py +++ b/src/ndev_settings/__init__.py @@ -1,7 +1,7 @@ -from ._settings import Settings +from ._settings import Settings, clear_settings from ._version import version as __version__ -__all__ = ["get_settings", "__version__"] +__all__ = ["get_settings", "clear_settings", "__version__"] # Singleton instance _settings_instance = None diff --git a/src/ndev_settings/_cli.py b/src/ndev_settings/_cli.py index a81240d..db29827 100644 --- a/src/ndev_settings/_cli.py +++ b/src/ndev_settings/_cli.py @@ -9,6 +9,9 @@ def reset_values_to_defaults(settings_file: Path | str) -> bool: """Reset all 'value' fields to match 'default' fields in a settings YAML file. + Works with flat format settings files where groups are at the top level. + Metadata keys starting with '_' (e.g., _entry_points_hash) are preserved. + Parameters ---------- settings_file : Path | str @@ -26,14 +29,20 @@ def reset_values_to_defaults(settings_file: Path | str) -> bool: return False with open(settings_file) as f: - settings = yaml.safe_load(f) + settings_data = yaml.load(f, Loader=yaml.FullLoader) - if not settings: + if not settings_data: return False modified = False - for group_name, group_settings in settings.items(): + for group_name, group_settings in settings_data.items(): + # Skip metadata keys (e.g., _entry_points_hash) + if group_name.startswith("_"): + continue + if not isinstance(group_settings, dict): + continue + for setting_name, setting_data in group_settings.items(): if ( isinstance(setting_data, dict) @@ -50,7 +59,9 @@ def reset_values_to_defaults(settings_file: Path | str) -> bool: if modified: with open(settings_file, "w") as f: - yaml.dump(settings, f, default_flow_style=False, sort_keys=False) + yaml.dump( + settings_data, f, default_flow_style=False, sort_keys=False + ) return modified diff --git a/src/ndev_settings/_settings.py b/src/ndev_settings/_settings.py index a1bcbb3..104d771 100644 --- a/src/ndev_settings/_settings.py +++ b/src/ndev_settings/_settings.py @@ -1,160 +1,314 @@ +from __future__ import annotations + +import hashlib +import logging from importlib.metadata import entry_points +from pathlib import Path +from urllib.parse import unquote, urlparse +import appdirs import yaml +logger = logging.getLogger(__name__) + +# User settings stored in platform-appropriate config directory +# Uses same location style as napari for familiarity +_SETTINGS_DIR = Path(appdirs.user_config_dir("ndev-settings", appauthor=False)) +_SETTINGS_FILE = _SETTINGS_DIR / "settings.yaml" + + +def _load_yaml(path: Path) -> dict: + """Load a YAML file, returning empty dict if missing or invalid. + + Uses FullLoader to support Python-specific types like tuples + (e.g., canvas_size: !!python/tuple [1024, 1024]) in settings files. + """ + try: + with open(path) as f: + return yaml.load(f, Loader=yaml.FullLoader) or {} + except (FileNotFoundError, yaml.YAMLError, OSError): + return {} + + +def _get_entry_points_hash() -> str: + """Generate a hash of installed ndev_settings.manifest entry points. + + Used to detect when packages are installed/removed. + """ + eps = entry_points(group="ndev_settings.manifest") + ep_strings = sorted(f"{ep.name}:{ep.value}" for ep in eps) + return hashlib.sha256("|".join(ep_strings).encode()).hexdigest() + + +def clear_settings() -> None: + """Clear saved settings. Next load will use fresh defaults.""" + if _SETTINGS_FILE.exists(): + _SETTINGS_FILE.unlink() + class SettingsGroup: """Simple container for settings in a group.""" class Settings: - """A class to manage settings for the nDev plugin, with nested group objects.""" + """Manages persistent settings for ndev plugins. - def __init__(self, settings_file: str): - self._settings_path = settings_file - self.load() + Settings are discovered from YAML files in installed packages (via entry points), + merged together, and stored in a single user settings file. User modifications + persist across sessions. - def reset_to_default( - self, setting_name: str | None = None, group: str | None = None - ): - """Reset a setting (or all settings) to their default values.""" - if setting_name: - # Reset single setting - find it in any group - for group_name, group_settings in self._grouped_settings.items(): - if setting_name in group_settings: - setting_data = group_settings[setting_name] - default_value = setting_data["default"] + When packages are installed/removed, new settings are merged in while + preserving existing user values. + """ - # Update both the SettingsGroup object and _grouped_settings - setattr( - getattr(self, group_name), setting_name, default_value - ) - setting_data["value"] = default_value + def __init__(self, defaults_file: str | None = None): + """Initialize settings. - self.save() - return + Parameters + ---------- + defaults_file : str, optional + Path to a YAML file with default settings. Usually the main + ndev_settings.yaml file. External contributions are merged in. + """ + self._defaults_path = Path(defaults_file) if defaults_file else None + self._grouped_settings: dict = {} + self._load() + + def _load(self): + """Load settings from saved file or discover from package YAML files. + + On first load (or after package changes), discovers settings from all + installed packages and saves them. Subsequent loads just read the saved file. + """ + saved = self._load_saved() + + if saved is not None: + # Fast path: use saved settings + all_settings = saved else: - # Reset all settings (optionally by group) - for group_name, group_settings in self._grouped_settings.items(): - if group and group_name != group: - continue - for name, setting_data in group_settings.items(): - if "default" in setting_data: - default_value = setting_data["default"] + # Slow path: discover from package YAML files, then save + all_settings = self._load_defaults() + self._save_settings(all_settings) - # Update both the SettingsGroup object and _grouped_settings - setattr(getattr(self, group_name), name, default_value) - setting_data["value"] = default_value - self.save() + # Build group objects and store + self._build_groups(all_settings) + self._grouped_settings = all_settings - def _get_dynamic_choices(self, provider_key: str) -> list: - """Get dynamic choices from entry points.""" - entries = entry_points(group=provider_key) - return [entry.name for entry in entries] + def _load_defaults(self) -> dict: + """Load default settings from main file and external contributions.""" + all_settings = {} - def load(self): - """Load settings from main file, external YAML files, and entry points.""" - # Start with main settings file - all_settings = self._load_yaml_file(self._settings_path) + # Load main defaults file if provided + if self._defaults_path: + all_settings = _load_yaml(self._defaults_path) # Load external YAML files from entry points - external_yaml_settings = self._load_external_yaml_files() - - # Merge external YAML settings - append new groups at the end to preserve order - # First, add settings to existing groups - for group_name, group_settings in external_yaml_settings.items(): - if group_name in all_settings: - # Add to existing group (main file settings take precedence) - for setting_name, setting_data in group_settings.items(): - if setting_name not in all_settings[group_name]: - all_settings[group_name][setting_name] = setting_data - - # Then, add completely new groups at the end - for group_name, group_settings in external_yaml_settings.items(): - if group_name not in all_settings: - all_settings[group_name] = group_settings - - # Create group objects from merged settings - for group_name, group_settings in all_settings.items(): - group_obj = SettingsGroup() - for name, setting_data in group_settings.items(): - if isinstance(setting_data, dict) and "value" in setting_data: - value = setting_data["value"] - setattr(group_obj, name, value) - setattr(self, group_name, group_obj) + # Sort for deterministic merge order across environments + eps = sorted( + entry_points(group="ndev_settings.manifest"), + key=lambda ep: (ep.name, ep.value), + ) + for ep in eps: + try: + package_name, resource_name = ep.value.split(":", 1) - self._grouped_settings = all_settings - # Don't auto-save during load - only save when explicitly requested - # This prevents modifying files during testing and keeps loading fast + # Use distribution() to find package location WITHOUT importing it + # This avoids slow package imports (e.g., ndevio takes 2.5s to import) + from importlib.metadata import distribution - def _load_yaml_file(self, yaml_path: str) -> dict: - """Load a single YAML settings file.""" - try: - with open(yaml_path) as file: - # Use FullLoader to support Python-specific tags like !!python/tuple - return yaml.load(file, Loader=yaml.FullLoader) or {} - except FileNotFoundError: - return {} - - def _load_external_yaml_files(self) -> dict: - """Load external YAML files from other packages via entry points.""" - all_external_settings = {} - # Look for entry points that provide YAML file paths - for entry_point in entry_points(group="ndev_settings.manifest"): - try: - # Support napari-style resource paths (e.g., "package:file.yaml") - entry_value = entry_point.value - - package_name, resource_name = entry_value.split(":", 1) - - from importlib.resources import files - - yaml_path = str(files(package_name) / resource_name) - external_settings = self._load_yaml_file(yaml_path) - # Merge with all external settings - for ( - group_name, - group_settings, - ) in external_settings.items(): - if group_name not in all_external_settings: - all_external_settings[group_name] = {} - all_external_settings[group_name].update(group_settings) + dist = distribution(package_name) + yaml_path = None + + # For regular installs, dist.files contains the file list + for file in dist.files or []: + if file.name == resource_name and package_name in str( + file + ): + yaml_path = Path(str(dist.locate_file(file))) + break + + if yaml_path is None: + # For editable installs, check direct_url.json (PEP 610) + # Find direct_url.json from dist.files (includes dist-info path) + direct_url_file = None + for file in dist.files or []: + if file.name == "direct_url.json": + direct_url_file = Path(str(dist.locate_file(file))) + break + + if ( + direct_url_file is not None + and direct_url_file.exists() + ): + import json + + with open(direct_url_file) as f: + direct_url = json.load(f) + if direct_url.get("dir_info", {}).get("editable"): + # Editable install - use source path + # Use urllib.parse for proper file URL handling + url = direct_url["url"] + parsed = urlparse(url) + if parsed.scheme == "file": + # Handle both Unix and Windows paths + path_str = unquote(parsed.path) + # On Windows, path may start with /C:/... + if ( + len(path_str) > 2 + and path_str[0] == "/" + and path_str[2] == ":" + ): + path_str = path_str[1:] # Remove leading / + source_path = Path(path_str) + else: + source_path = Path(url) + yaml_path = ( + source_path + / "src" + / package_name + / resource_name + ) + + if yaml_path is None: + # Final fallback: try site-packages path + package_location = str(dist.locate_file(package_name)) + yaml_path = Path(package_location) / resource_name + + external = _load_yaml(yaml_path) + + # Merge external settings (first one wins for conflicts) + for group_name, group_settings in external.items(): + if group_name not in all_settings: + all_settings[group_name] = {} + for name, data in group_settings.items(): + if name not in all_settings[group_name]: + all_settings[group_name][name] = data except ( ModuleNotFoundError, FileNotFoundError, - AttributeError, ValueError, + PermissionError, + OSError, ) as e: - print( - f"Failed to load external settings from entry point '{entry_point.name}': {e}" + logger.warning( + "Failed to load settings from '%s': %s", ep.name, e ) - return all_external_settings + return all_settings - def save(self): - """Save the current state of all settings to the YAML file, preserving original order.""" - # Update the _grouped_settings values from the current group objects - for group_name, group_settings in self._grouped_settings.items(): - if hasattr(self, group_name): - group_obj = getattr(self, group_name) - if isinstance(group_obj, SettingsGroup): - for setting_name in group_settings: - if hasattr(group_obj, setting_name): - current_value = getattr(group_obj, setting_name) - # Update the value in place - self._grouped_settings[group_name][setting_name][ - "value" - ] = current_value - - # Save the updated _grouped_settings directly - self._save_settings_file(self._grouped_settings) - - def _save_settings_file(self, settings_data): - """Helper to save settings data to file.""" - with open(self._settings_path, "w") as file: - yaml.dump( - settings_data, - file, - default_flow_style=False, - sort_keys=False, + def _load_saved(self) -> dict | None: + """Load saved settings if valid, return None if stale or missing.""" + if not _SETTINGS_FILE.exists(): + return None + + saved = _load_yaml(_SETTINGS_FILE) + if not saved: + return None + + # Check if packages changed - if so, need to re-discover + # Pop the hash from saved - this is safe because: + # - If hash doesn't match, we merge with defaults (saved used for values only) + # - If hash matches, we return saved (but _build_groups skips _ keys anyway) + saved_hash = saved.pop("_entry_points_hash", None) + if saved_hash != _get_entry_points_hash(): + # Packages installed/removed - merge new defaults with saved values + defaults = self._load_defaults() + merged = self._merge_with_saved(defaults, saved) + self._save_settings(merged) + return merged + + return saved + + def _merge_with_saved(self, defaults: dict, saved: dict) -> dict: + """Merge saved user values into fresh defaults.""" + merged = {} + for group_name, group_settings in defaults.items(): + # Skip metadata keys + if group_name.startswith("_"): + continue + merged[group_name] = {} + for name, data in group_settings.items(): + merged[group_name][name] = data.copy() + # Override with saved value if exists + if group_name in saved and name in saved[group_name]: + saved_data = saved[group_name][name] + if isinstance(saved_data, dict) and "value" in saved_data: + merged[group_name][name]["value"] = saved_data["value"] + return merged + + def _save_settings(self, settings: dict) -> None: + """Save settings to file in flat format with hash at top.""" + try: + _SETTINGS_DIR.mkdir(parents=True, exist_ok=True) + # Flat format: hash first, then all settings groups + data = {"_entry_points_hash": _get_entry_points_hash(), **settings} + with open(_SETTINGS_FILE, "w") as f: + yaml.dump(data, f, default_flow_style=False, sort_keys=False) + except (OSError, PermissionError) as e: + logger.warning( + "Failed to save settings to %s: %s", _SETTINGS_FILE, e ) + + def _build_groups(self, settings: dict): + """Create SettingsGroup objects from settings dict.""" + for group_name, group_settings in settings.items(): + # Skip metadata keys (e.g., _entry_points_hash) + if group_name.startswith("_"): + continue + # Skip malformed entries that aren't dicts + if not isinstance(group_settings, dict): + continue + group_obj = SettingsGroup() + for name, setting_data in group_settings.items(): + if isinstance(setting_data, dict) and "value" in setting_data: + setattr(group_obj, name, setting_data["value"]) + setattr(self, group_name, group_obj) + + def _sync_groups_to_dict(self): + """Sync current group object values back to _grouped_settings dict.""" + for group_name, group_settings in self._grouped_settings.items(): + group_obj = getattr(self, group_name, None) + if isinstance(group_obj, SettingsGroup): + for setting_name in group_settings: + if hasattr(group_obj, setting_name): + value = getattr(group_obj, setting_name) + self._grouped_settings[group_name][setting_name][ + "value" + ] = value + + def save(self): + """Save current settings to persist across sessions.""" + self._sync_groups_to_dict() + self._save_settings(self._grouped_settings) + + def reset_to_default( + self, setting_name: str | None = None, group: str | None = None + ): + """Reset a setting (or all settings) to their default values.""" + if setting_name: + # Reset single setting + for group_name, group_settings in self._grouped_settings.items(): + if setting_name in group_settings: + default = group_settings[setting_name].get("default") + if default is not None: + setattr( + getattr(self, group_name), setting_name, default + ) + group_settings[setting_name]["value"] = default + self.save() + return + else: + # Reset all settings (optionally filtered by group) + for group_name, group_settings in self._grouped_settings.items(): + if group and group_name != group: + continue + for name, setting_data in group_settings.items(): + if "default" in setting_data: + default = setting_data["default"] + setattr(getattr(self, group_name), name, default) + setting_data["value"] = default + self.save() + + def get_dynamic_choices(self, provider_key: str) -> list: + """Get dynamic choices from entry points.""" + return [ep.name for ep in entry_points(group=provider_key)] diff --git a/src/ndev_settings/_settings_widget.py b/src/ndev_settings/_settings_widget.py index 322a5d4..00d3ff1 100644 --- a/src/ndev_settings/_settings_widget.py +++ b/src/ndev_settings/_settings_widget.py @@ -21,7 +21,7 @@ def _get_dynamic_choices(self, setting_info: dict) -> tuple[list, str]: "fallback_message", "No choices available" ) - choices = self.settings._get_dynamic_choices(provider) + choices = self.settings.get_dynamic_choices(provider) return choices if choices else [fallback_message], fallback_message def _create_widget_for_setting( @@ -45,8 +45,7 @@ def _create_widget_for_setting( for key, value in info.items(): if key in ["default", "value", "tooltip", "dynamic_choices"]: continue - else: - widget_options[key] = value + widget_options[key] = value # Handle dynamic choices if "dynamic_choices" in info: diff --git a/tests/conftest.py b/tests/conftest.py index e1207a4..35dcef5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,46 +7,59 @@ @pytest.fixture(autouse=True) -def mock_settings_file_path(tmp_path, monkeypatch, test_data_dir): - """Mock the settings file path to use test data during tests.""" - from pathlib import Path +def isolate_settings(tmp_path, monkeypatch, test_data_dir): + """Isolate settings to tmp_path for each test. + This redirects both: + 1. The settings file location (where user settings are saved) + 2. The defaults file (to use test data instead of real package defaults) + 3. Entry points (to prevent external packages from contributing settings) + + This allows tests to run with real persistence behavior while being isolated. + """ import ndev_settings + from ndev_settings import _settings from ndev_settings._settings import Settings - # Use the existing test_settings.yaml as our mock data - test_settings_path = tmp_path / "test_ndev_settings.yaml" - shutil.copy(test_data_dir / "test_settings.yaml", test_settings_path) + # Redirect settings file to tmp_path + test_settings_dir = tmp_path / "ndev-settings" + test_settings_file = test_settings_dir / "settings.yaml" + monkeypatch.setattr(_settings, "_SETTINGS_DIR", test_settings_dir) + monkeypatch.setattr(_settings, "_SETTINGS_FILE", test_settings_file) - # Store the original Settings class constructor - original_settings_init = Settings.__init__ + # Mock entry_points to return empty list (isolate from external packages) + from importlib.metadata import EntryPoints - # Get the real default settings file path - real_settings_path = str( - Path(ndev_settings.__file__).parent / "ndev_settings.yaml" + monkeypatch.setattr( + _settings, + "entry_points", + lambda group: EntryPoints([]), ) - def mock_settings_init(self, settings_file=None): - """Mock Settings constructor to use test file only when loading the default settings.""" - # Only redirect if this is the default ndev_settings.yaml path - if settings_file == real_settings_path: - settings_file = str(test_settings_path) + # Copy test data to use as defaults + test_defaults_path = tmp_path / "test_defaults.yaml" + shutil.copy(test_data_dir / "test_settings.yaml", test_defaults_path) - # Ensure settings_file is never None - if settings_file is None: - settings_file = str(test_settings_path) + # Redirect default settings file path + original_settings_init = Settings.__init__ + real_settings_path = str( + Path(ndev_settings.__file__).parent / "ndev_settings.yaml" + ) - return original_settings_init(self, settings_file) + def mock_settings_init(self, defaults_file=None): + """Redirect default settings file to test data.""" + if defaults_file == real_settings_path or defaults_file is None: + defaults_file = str(test_defaults_path) + return original_settings_init(self, defaults_file) - # Mock the Settings class constructor monkeypatch.setattr(Settings, "__init__", mock_settings_init) - # Reset singleton before test + # Reset singleton before each test ndev_settings._settings_instance = None - yield test_settings_path + yield test_defaults_path - # Clean up + # Clean up singleton after test ndev_settings._settings_instance = None @@ -82,7 +95,7 @@ def empty_settings_file(tmp_path): @pytest.fixture def mock_external_contributions(tmp_path, test_data_dir, monkeypatch): - """Mock entry points that provide external YAML contributions using napari-style resource paths.""" + """Mock entry points that provide external YAML contributions.""" # Copy external contribution file to temp directory external_file = tmp_path / "external_contribution.yaml" @@ -93,6 +106,8 @@ def __init__(self, name, package_name, resource_name, resource_path): self.name = name self.value = f"{package_name}:{resource_name}" self._resource_path = resource_path + self._package_name = package_name + self._resource_name = resource_name def mock_entry_points(group=None): if group == "ndev_settings.manifest": @@ -106,23 +121,46 @@ def mock_entry_points(group=None): ] return [] - # Mock importlib.resources.files to return our test file - def mock_files(package_name): - if package_name == "mock_package": + # Mock distribution() to return our test file path + class MockPackagePath: + def __init__(self, package_name, resource_name, actual_path): + self._path = actual_path + self.name = resource_name # This is what file.name returns + self._package_name = package_name + + def __str__(self): + # Return something like "mock_package/settings.yaml" + # so package_name is in str(file) + return f"{self._package_name}/{self.name}" + + class MockDistribution: + def __init__(self, package_name, resource_name, resource_path): + self._package_name = package_name + self._resource_path = resource_path + # Create a mock files list with proper name and str representation + self.files = [ + MockPackagePath(package_name, resource_name, resource_path) + ] + + def locate_file(self, file): + if hasattr(file, "_path"): + return file._path + # For editable install fallback + return self._resource_path.parent - class MockPath: - def __truediv__(self, resource_name): - if resource_name == "settings.yaml": - return external_file - return tmp_path / resource_name + from importlib.metadata import distribution as orig_dist - return MockPath() - raise ImportError(f"No module named '{package_name}'") + def mock_distribution(package_name): + if package_name == "mock_package": + return MockDistribution( + package_name, "settings.yaml", external_file + ) + return orig_dist(package_name) # Apply the patches monkeypatch.setattr( "ndev_settings._settings.entry_points", mock_entry_points ) - monkeypatch.setattr("importlib.resources.files", mock_files) + monkeypatch.setattr("importlib.metadata.distribution", mock_distribution) return external_file diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 0000000..9e44aa3 --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,298 @@ +"""Tests for ndev-settings CLI utilities.""" + +import sys + +import yaml + +from ndev_settings._cli import main_reset_values, reset_values_to_defaults + + +class TestResetValuesToDefaults: + """Tests for reset_values_to_defaults function.""" + + def test_reset_modified_values(self, tmp_path): + """Test that modified values are reset to defaults.""" + settings_file = tmp_path / "settings.yaml" + settings_file.write_text( + yaml.dump( + { + "TestGroup": { + "setting1": { + "value": "modified", + "default": "original", + }, + "setting2": {"value": 100, "default": 50}, + } + } + ) + ) + + result = reset_values_to_defaults(settings_file) + + assert result is True + with open(settings_file) as f: + updated = yaml.load(f, Loader=yaml.FullLoader) + assert updated["TestGroup"]["setting1"]["value"] == "original" + assert updated["TestGroup"]["setting2"]["value"] == 50 + + def test_no_changes_when_values_match_defaults(self, tmp_path): + """Test that no changes are made when values already match defaults.""" + settings_file = tmp_path / "settings.yaml" + settings_file.write_text( + yaml.dump( + { + "TestGroup": { + "setting1": { + "value": "original", + "default": "original", + }, + } + } + ) + ) + + result = reset_values_to_defaults(settings_file) + + assert result is False + + def test_missing_file_returns_false(self, tmp_path, capsys): + """Test that missing file returns False and prints message.""" + missing_file = tmp_path / "nonexistent.yaml" + + result = reset_values_to_defaults(missing_file) + + assert result is False + captured = capsys.readouterr() + assert "Settings file not found" in captured.out + + def test_empty_file_returns_false(self, tmp_path): + """Test that empty file returns False.""" + settings_file = tmp_path / "empty.yaml" + settings_file.write_text("") + + result = reset_values_to_defaults(settings_file) + + assert result is False + + def test_settings_without_default_key_unchanged(self, tmp_path): + """Test that settings without 'default' key are not modified.""" + settings_file = tmp_path / "settings.yaml" + settings_file.write_text( + yaml.dump( + { + "TestGroup": { + "setting_no_default": {"value": "something"}, + } + } + ) + ) + + result = reset_values_to_defaults(settings_file) + + assert result is False + + def test_settings_without_value_key_unchanged(self, tmp_path): + """Test that settings without 'value' key are not modified.""" + settings_file = tmp_path / "settings.yaml" + settings_file.write_text( + yaml.dump( + { + "TestGroup": { + "setting_no_value": {"default": "something"}, + } + } + ) + ) + + result = reset_values_to_defaults(settings_file) + + assert result is False + + def test_accepts_string_path(self, tmp_path): + """Test that string paths are accepted.""" + settings_file = tmp_path / "settings.yaml" + settings_file.write_text( + yaml.dump( + { + "TestGroup": { + "setting1": { + "value": "modified", + "default": "original", + }, + } + } + ) + ) + + result = reset_values_to_defaults(str(settings_file)) + + assert result is True + + def test_flat_format_with_entry_points_hash(self, tmp_path): + """Test handling of cached settings file with _entry_points_hash.""" + settings_file = tmp_path / "settings.yaml" + settings_file.write_text( + yaml.dump( + { + "_entry_points_hash": "abc123", + "TestGroup": { + "setting1": { + "value": "modified", + "default": "original", + }, + "setting2": {"value": 100, "default": 50}, + }, + } + ) + ) + + result = reset_values_to_defaults(settings_file) + + assert result is True + with open(settings_file) as f: + updated = yaml.load(f, Loader=yaml.FullLoader) + # Hash should be preserved + assert updated["_entry_points_hash"] == "abc123" + # Settings should be reset + assert updated["TestGroup"]["setting1"]["value"] == "original" + assert updated["TestGroup"]["setting2"]["value"] == 50 + + def test_skips_underscore_prefixed_keys(self, tmp_path): + """Test that underscore-prefixed keys are skipped, not treated as groups.""" + settings_file = tmp_path / "settings.yaml" + settings_file.write_text( + yaml.dump( + { + "_metadata": "some_value", # Should be skipped + "TestGroup": { + "setting1": { + "value": "modified", + "default": "original", + }, + }, + } + ) + ) + + # Should not raise an error when encountering non-dict group + result = reset_values_to_defaults(settings_file) + + assert result is True + + +class TestMainResetValues: + """Tests for main_reset_values CLI entry point.""" + + def test_no_args_shows_usage(self, monkeypatch, capsys): + """Test that no arguments shows usage and returns 1.""" + monkeypatch.setattr(sys, "argv", ["reset-settings-values"]) + + result = main_reset_values() + + assert result == 1 + captured = capsys.readouterr() + assert "Usage:" in captured.out + assert "pre-commit hook" in captured.out + + def test_single_file_modified(self, tmp_path, monkeypatch, capsys): + """Test processing a single file that needs modification.""" + settings_file = tmp_path / "settings.yaml" + settings_file.write_text( + yaml.dump( + { + "TestGroup": { + "setting1": { + "value": "modified", + "default": "original", + }, + } + } + ) + ) + monkeypatch.setattr( + sys, "argv", ["reset-settings-values", str(settings_file)] + ) + + result = main_reset_values() + + assert result == 1 + captured = capsys.readouterr() + assert "WARNING" in captured.out + assert "re-stage" in captured.out + + def test_single_file_unchanged(self, tmp_path, monkeypatch): + """Test processing a single file that doesn't need modification.""" + settings_file = tmp_path / "settings.yaml" + settings_file.write_text( + yaml.dump( + { + "TestGroup": { + "setting1": { + "value": "original", + "default": "original", + }, + } + } + ) + ) + monkeypatch.setattr( + sys, "argv", ["reset-settings-values", str(settings_file)] + ) + + result = main_reset_values() + + assert result == 0 + + def test_multiple_files(self, tmp_path, monkeypatch, capsys): + """Test processing multiple files.""" + file1 = tmp_path / "settings1.yaml" + file2 = tmp_path / "settings2.yaml" + file1.write_text( + yaml.dump( + { + "Group1": { + "s1": {"value": "modified", "default": "original"} + } + } + ) + ) + file2.write_text( + yaml.dump( + { + "Group2": { + "s2": {"value": "original", "default": "original"} + } + } + ) + ) + monkeypatch.setattr( + sys, "argv", ["reset-settings-values", str(file1), str(file2)] + ) + + result = main_reset_values() + + assert result == 1 # At least one file was modified + captured = capsys.readouterr() + assert "Resetting Group1.s1" in captured.out + + def test_missing_file_in_list(self, tmp_path, monkeypatch, capsys): + """Test that missing files are handled gracefully.""" + existing_file = tmp_path / "exists.yaml" + existing_file.write_text( + yaml.dump( + {"Group": {"s": {"value": "original", "default": "original"}}} + ) + ) + missing_file = tmp_path / "missing.yaml" + + monkeypatch.setattr( + sys, + "argv", + ["reset-settings-values", str(existing_file), str(missing_file)], + ) + + result = main_reset_values() + + assert result == 0 # No modifications made + captured = capsys.readouterr() + assert "not found" in captured.out diff --git a/tests/test_settings.py b/tests/test_settings.py index e22d339..756b5d9 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -99,23 +99,133 @@ def test_reset_by_group(self, test_settings_file): class TestSettingsSaveLoad: """Test Settings save and load functionality.""" - def test_save_and_reload(self, test_settings_file): - """Test saving changes and reloading them.""" + def test_save_syncs_values(self, test_settings_file): + """Test that save() syncs group values to internal dict.""" settings = Settings(str(test_settings_file)) - # Modify some settings + # Modify some settings via group objects settings.Group_A.setting_int = 555 settings.Group_A.setting_string = "Saved text" - # Save + # Before save, _grouped_settings should still have old values + assert ( + settings._grouped_settings["Group_A"]["setting_int"]["value"] + != 555 + ) + + # Save syncs values settings.save() - # Create new Settings instance from same file + # Now internal dict should be updated + assert ( + settings._grouped_settings["Group_A"]["setting_int"]["value"] + == 555 + ) + assert ( + settings._grouped_settings["Group_A"]["setting_string"]["value"] + == "Saved text" + ) + + def test_save_persists_across_instances(self, test_settings_file): + """Test that saved settings are loaded by new instances.""" + settings1 = Settings(str(test_settings_file)) + + # Modify and save + settings1.Group_A.setting_int = 999 + settings1.save() + + # Create new instance - should load saved value + settings2 = Settings(str(test_settings_file)) + assert settings2.Group_A.setting_int == 999 + + def test_cached_load_uses_saved_file( + self, test_settings_file, monkeypatch + ): + """Test that second load uses cached settings file, not _load_defaults.""" + from ndev_settings import _settings + + # Track all calls to _load_defaults + load_defaults_call_count = 0 + original_load_defaults = Settings._load_defaults + + def tracking_load_defaults(self): + nonlocal load_defaults_call_count + load_defaults_call_count += 1 + return original_load_defaults(self) + + monkeypatch.setattr(Settings, "_load_defaults", tracking_load_defaults) + + # First load - should discover from files (calls _load_defaults) + settings1 = Settings(str(test_settings_file)) + first_load_calls = load_defaults_call_count + + # Verify the cache file was created + assert ( + _settings._SETTINGS_FILE.exists() + ), "Cache file should exist after first load" + + # Second load - should use cached file, not call _load_defaults again + settings2 = Settings(str(test_settings_file)) + + # Both should have same values + assert settings1.Group_A.setting_int == settings2.Group_A.setting_int + assert settings2._grouped_settings is not None + + # _load_defaults should have been called once (first load), but not on second + assert ( + load_defaults_call_count == first_load_calls + ), f"Expected {first_load_calls} calls, got {load_defaults_call_count} - cache not working" + + +class TestCachingBehavior: + """Test settings caching and persistence behavior.""" + + def test_clear_settings_forces_rediscovery(self, test_settings_file): + """Test that clear_settings() forces re-discovery from defaults.""" + from ndev_settings import _settings + + settings1 = Settings(str(test_settings_file)) + settings1.Group_A.setting_int = 888 + settings1.save() + + # Clear settings + _settings.clear_settings() + + # New instance should have default values settings2 = Settings(str(test_settings_file)) + assert ( + settings2.Group_A.setting_int == 49 + ) # default from test_settings.yaml - # Should have saved changes - assert settings2.Group_A.setting_int == 555 - assert settings2.Group_A.setting_string == "Saved text" + def test_package_change_preserves_user_values( + self, test_settings_file, monkeypatch + ): + """Test that when packages change, user values are preserved.""" + from ndev_settings import _settings + + # First load and save custom values + settings1 = Settings(str(test_settings_file)) + settings1.Group_A.setting_int = 777 + settings1.save() + + # Simulate a package change by modifying the hash function + _ = _settings._get_entry_points_hash() + monkeypatch.setattr( + _settings, "_get_entry_points_hash", lambda: "different_hash" + ) + + # New instance should merge: new defaults + saved user values + settings2 = Settings(str(test_settings_file)) + + # User's custom value should be preserved + assert settings2.Group_A.setting_int == 777 + + def test_clear_settings_handles_missing_file(self): + """Test that clear_settings doesn't crash if file doesn't exist.""" + from ndev_settings import _settings + + # This should not raise even if file doesn't exist + _settings.clear_settings() class TestDynamicChoices: @@ -126,13 +236,13 @@ def test_dynamic_choices_handling(self, test_settings_file): settings = Settings(str(test_settings_file)) # Test the dynamic choices method - choices = settings._get_dynamic_choices("bioio.readers") + choices = settings.get_dynamic_choices("bioio.readers") # Should return a list (even if empty) assert isinstance(choices, list) # Test with invalid provider - empty_choices = settings._get_dynamic_choices("invalid.provider") + empty_choices = settings.get_dynamic_choices("invalid.provider") assert empty_choices == [] @@ -188,18 +298,22 @@ def test_external_adds_new_setting_to_existing_group( def test_external_settings_can_be_modified( self, test_settings_file, mock_external_contributions ): - """Test that external settings can be modified and saved.""" + """Test that external settings can be modified via group objects.""" settings = Settings(str(test_settings_file)) - # Modify external setting + # Modify external setting via group object settings.External_Contribution.setting_int = 999 - # Save and reload + # Save syncs the value to internal dict settings.save() - settings2 = Settings(str(test_settings_file)) - # Should preserve the change - assert settings2.External_Contribution.setting_int == 999 + # Verify the internal dict was updated + assert ( + settings._grouped_settings["External_Contribution"]["setting_int"][ + "value" + ] + == 999 + ) def test_duplicate_external_contributions_handling( self, test_settings_file, tmp_path, monkeypatch @@ -245,7 +359,7 @@ def test_duplicate_external_contributions_handling( yaml.dump(external2_data, default_flow_style=False) ) - # Mock multiple entry points using napari-style resource paths + # Mock multiple entry points class MockEntryPoint: def __init__( self, name, package_name, resource_name, resource_path @@ -272,29 +386,49 @@ def mock_entry_points(group=None): ] return [] - # Mock importlib.resources.files to return our test files - def mock_files(package_name): - class MockPath: - def __truediv__(self, resource_name): - if ( - package_name == "mock_package1" - and resource_name == "settings.yaml" - ): - return external1_file - elif ( - package_name == "mock_package2" - and resource_name == "settings.yaml" - ): - return external2_file - return tmp_path / resource_name - - return MockPath() + # Mock distribution() to return our test files + class MockPackagePath: + def __init__(self, package_name, resource_name, actual_path): + self._path = actual_path + self.name = resource_name + self._package_name = package_name + + def __str__(self): + return f"{self._package_name}/{self.name}" + + class MockDistribution: + def __init__(self, package_name, resource_name, resource_path): + self._package_name = package_name + self._resource_path = resource_path + self.files = [ + MockPackagePath(package_name, resource_name, resource_path) + ] + + def locate_file(self, file): + if hasattr(file, "_path"): + return file._path + return self._resource_path.parent + + from importlib.metadata import distribution as orig_dist + + def mock_distribution(package_name): + if package_name == "mock_package1": + return MockDistribution( + package_name, "settings.yaml", external1_file + ) + elif package_name == "mock_package2": + return MockDistribution( + package_name, "settings.yaml", external2_file + ) + return orig_dist(package_name) # Apply patches monkeypatch.setattr( "ndev_settings._settings.entry_points", mock_entry_points ) - monkeypatch.setattr("importlib.resources.files", mock_files) + monkeypatch.setattr( + "importlib.metadata.distribution", mock_distribution + ) # Load settings settings = Settings(str(test_settings_file)) @@ -302,11 +436,11 @@ def __truediv__(self, resource_name): # Should have the shared group assert hasattr(settings, "Shared_Group") - # Last external contribution should win (external2) - # because _load_external_yaml_files uses dict.update() which overwrites existing keys - assert settings.Shared_Group.shared_setting == "from_external2" + # First external contribution wins for overlapping settings (stable, predictable) + # This prevents unpredictable behavior based on package load order + assert settings.Shared_Group.shared_setting == "from_external1" - # But unique settings from later externals should still be added + # Unique settings from later externals should still be added assert settings.Shared_Group.unique_setting == "unique_to_external2" @@ -369,6 +503,141 @@ def test_dynamic_choices(empty_settings_file): ) # Create without calling __init__ # Test the method exists and handles missing entry points gracefully - choices = settings._get_dynamic_choices("nonexistent.entry.point") + choices = settings.get_dynamic_choices("nonexistent.entry.point") assert isinstance(choices, list) assert len(choices) == 0 # Should return empty list when no entries found + + +class TestEdgeCases: + """Test edge cases and error handling.""" + + def test_build_groups_skips_metadata_keys(self, test_settings_file): + """Test that _build_groups skips underscore-prefixed keys.""" + settings = Settings(str(test_settings_file)) + + # Manually call _build_groups with settings containing metadata + settings_with_metadata = { + "_entry_points_hash": "abc123", + "_other_metadata": {"some": "data"}, + "ValidGroup": {"setting1": {"value": 10, "default": 10}}, + } + + # Clear existing groups and rebuild + settings._build_groups(settings_with_metadata) + + # Should have ValidGroup but not metadata keys as attributes + assert hasattr(settings, "ValidGroup") + assert not hasattr(settings, "_entry_points_hash") + assert not hasattr(settings, "_other_metadata") + + def test_save_failure_logs_warning( + self, test_settings_file, monkeypatch, caplog + ): + """Test that save failures are logged as warnings.""" + import logging + + settings = Settings(str(test_settings_file)) + + # Mock yaml.dump to raise when trying to save + def failing_dump(*args, **kwargs): + raise OSError("Cannot write to file") + + monkeypatch.setattr("yaml.dump", failing_dump) + + # Should not raise, but should log + with caplog.at_level(logging.WARNING): + settings._save_settings(settings._grouped_settings) + + assert "Failed to save settings" in caplog.text + + def test_editable_install_detection( + self, test_settings_file, tmp_path, monkeypatch + ): + """Test that editable installs are detected via direct_url.json.""" + import json + + from ndev_settings import _settings + + # Create a mock editable install structure + source_dir = tmp_path / "source" + source_dir.mkdir() + src_package_dir = source_dir / "src" / "mock_package" + src_package_dir.mkdir(parents=True) + yaml_file = src_package_dir / "settings.yaml" + yaml_file.write_text( + "Editable_Group:\n setting1:\n value: 42\n default: 42\n" + ) + + # Create a mock dist_info with direct_url.json + dist_info_path = tmp_path / "mock_package-1.0.0.dist-info" + dist_info_path.mkdir() + direct_url_file = dist_info_path / "direct_url.json" + direct_url_file.write_text( + json.dumps( + { + "url": f"file:///{source_dir.as_posix()}", + "dir_info": {"editable": True}, + } + ) + ) + + class MockEntryPoint: + def __init__(self): + self.name = "mock_editable" + self.value = "mock_package:settings.yaml" + + class MockFile: + """Mock file object that mimics PackagePath behavior.""" + + def __init__(self, path): + self._path = path + + @property + def name(self): + return self._path.split("/")[-1] + + def __str__(self): + return self._path + + class MockDistribution: + def __init__(self): + self._path = dist_info_path + # Include direct_url.json in the files list (like real dist-info) + self.files = [ + MockFile("mock_package-1.0.0.dist-info/direct_url.json") + ] + + def locate_file(self, file): + # Handle both string and MockFile input + file_str = str(file) + if "direct_url.json" in file_str: + return direct_url_file + return tmp_path / "nonexistent" # Force fallback for others + + def mock_entry_points(group=None): + if group == "ndev_settings.manifest": + return [MockEntryPoint()] + return [] + + def mock_distribution(name): + if name == "mock_package": + return MockDistribution() + from importlib.metadata import distribution + + return distribution(name) + + monkeypatch.setattr(_settings, "entry_points", mock_entry_points) + monkeypatch.setattr( + "importlib.metadata.distribution", mock_distribution + ) + + # Clear any cached settings to force fresh load + _settings.clear_settings() + + settings = Settings(str(test_settings_file)) + + # Should have loaded the editable package's settings + assert hasattr( + settings, "Editable_Group" + ), f"Missing Editable_Group. Attrs: {[a for a in dir(settings) if not a.startswith('_')]}" + assert settings.Editable_Group.setting1 == 42 diff --git a/tox.ini b/tox.ini index 1f60aaa..b736b0c 100644 --- a/tox.ini +++ b/tox.ini @@ -29,5 +29,5 @@ passenv = NUMPY_EXPERIMENTAL_ARRAY_FUNCTION PYVISTA_OFF_SCREEN dependency_groups = - testing + dev commands = pytest -v --color=yes --cov=ndev_settings --cov-report=xml