Skip to content

refactor: Remove * imports and update __all__ to be static#6802

Merged
hoxbro merged 29 commits into
mainfrom
refactor_no_star_with_all
Mar 4, 2026
Merged

refactor: Remove * imports and update __all__ to be static#6802
hoxbro merged 29 commits into
mainfrom
refactor_no_star_with_all

Conversation

@hoxbro

@hoxbro hoxbro commented Feb 25, 2026

Copy link
Copy Markdown
Member

Description

This PR enables the lint rules to not have * imports and make __all__ behave static with strings.

script used to generate all.json

Details

#!/usr/bin/env python3
"""
Collect __all__ from __init__.py files of a package and all subpackages.

Usage:
    python collect_init_all.py output.json holoviews
"""

import importlib
import json
import pkgutil
import sys
from types import ModuleType
from typing import Any


def get_all_value(module: ModuleType) -> Any:
    if hasattr(module, "__all__"):
        value = module.__all__
        if isinstance(value, (list, tuple)):
            return sorted(value)
        return value
    return None


def collect_package_inits(package_name: str) -> dict[str, Any]:
    results: dict[str, Any] = {}

    try:
        package = importlib.import_module(package_name)
    except Exception as e:
        return {package_name: {"error": str(e)}}

    # Must be a package
    if not hasattr(package, "__path__"):
        return {package_name: {"error": "Not a package"}}

    # Top-level package (__init__.py)
    results[package_name] = get_all_value(package)

    # Walk only subpackages (ispkg=True)
    for module_info in pkgutil.walk_packages(
        package.__path__, prefix=package.__name__ + "."
    ):
        if module_info.ispkg:
            try:
                subpkg = importlib.import_module(module_info.name)
                results[module_info.name] = get_all_value(subpkg)
            except Exception as e:
                results[module_info.name] = {"error": str(e)}

    return results


def main() -> None:
    if len(sys.argv) != 3:
        print("Usage: python collect_init_all.py output.json package")
        sys.exit(1)

    output_file = sys.argv[1]
    package_name = sys.argv[2]

    result = collect_package_inits(package_name)

    with open(output_file, "w", encoding="utf-8") as f:
        json.dump(result, f, indent=2, sort_keys=True)
        f.write("\n")

    print(f"Saved results to {output_file}")


if __name__ == "__main__":
    main()

Script to check for re-import problems.

Details

#!/usr/bin/env python3
"""
Verify that all `from holoviews... import X` imports
come from the module where X is actually defined.

- Only checks holoviews imports
- Resolves relative imports
- Skips `import holoviews`
- Fails on re-exports
"""

from __future__ import annotations

import ast
import importlib
import importlib.util
import pathlib
import sys
from collections.abc import Iterable
from types import ModuleType

PROJECT_ROOT = pathlib.Path(sys.argv[1] if len(sys.argv) > 1 else ".").resolve() / "holoviews"


def iter_python_files(root: pathlib.Path) -> Iterable[pathlib.Path]:
    for path in root.rglob("*.py"):
        if "site-packages" in path.parts:
            continue
        if "tests" in path.parts:
            continue
        yield path


def resolve_module(name: str) -> ModuleType:
    spec = importlib.util.find_spec(name)
    if spec is None:
        raise ImportError(f"Cannot resolve module '{name}'")
    return importlib.import_module(name)


def verify_symbol(module_name: str, symbol: str, file: pathlib.Path, lineno: int):
    try:
        module = resolve_module(module_name)
    except ImportError:
        return

    if not hasattr(module, symbol):
        raise RuntimeError(
            f"{file}:{lineno}: '{symbol}' not found in '{module_name}'"
        )

    obj = getattr(module, symbol)

    # Skip constants (no __module__ attribute)
    if not hasattr(obj, "__module__"):
        return

    origin = obj.__module__

    # Skip if the origin is outside holoviews (e.g. stdlib/third-party types)
    if not origin.startswith("holoviews"):
        return

    # Skip if the origin is a private module (intentional re-export)
    if any(part.startswith("_") for part in origin.split(".")):
        return

    # Skip if origin is a direct child of the import target (re-export via __init__)
    prefix = module_name + "."
    if origin.startswith(prefix) and "." not in origin[len(prefix):]:
        return


    if origin != module_name:
        raise RuntimeError(
            f"{file}:{lineno}: "
            f"'{symbol}' imported from '{module_name}' "
            f"but defined in '{origin}'"
        )


def file_to_package(path: pathlib.Path) -> str:
    """Convert a file path to its containing package name."""
    rel = path.relative_to(PROJECT_ROOT.parent)
    parts = list(rel.with_suffix("").parts)
    # For __init__.py the package is the directory itself,
    # for other files the package is the parent directory.
    if parts[-1] == "__init__":
        parts.pop()
    else:
        parts.pop()
    return ".".join(parts)


def resolve_relative_import(package: str, module: str | None, level: int) -> str:
    """Resolve a relative import to an absolute module name."""
    parts = package.split(".")
    # level=1 means current package, level=2 means parent, etc.
    parts = parts[: len(parts) - (level - 1)]
    base = ".".join(parts)
    if module:
        return f"{base}.{module}"
    return base


def process_file(path: pathlib.Path):
    tree = ast.parse(path.read_text(), filename=str(path))
    package = file_to_package(path)

    for node in ast.walk(tree):
        if not isinstance(node, ast.ImportFrom):
            continue

        if node.level > 0:
            # Resolve relative import
            module_name = resolve_relative_import(package, node.module, node.level)
        elif not node.module:
            continue
        else:
            module_name = node.module

        # Only care about holoviews
        if not module_name.startswith("holoviews"):
            continue

        for alias in node.names:
            if alias.name == "*":
                continue

            verify_symbol(
                module_name,
                alias.name,
                path,
                node.lineno,
            )


def main():
    errors = []

    for file in iter_python_files(PROJECT_ROOT):
        try:
            process_file(file)
        except Exception as e:
            errors.append(str(e))

    if errors:
        print("\n".join(dict.fromkeys(errors)))
        sys.exit(1)

    print("All holoviews imports are direct definitions ✓")


if __name__ == "__main__":
    main()

Checklist

  • Pull request title follows the conventional format
  • Tests added and is passing

Comment thread doc/conf.py
param.parameterized.docstring_signature = False
param.parameterized.docstring_describe_params = False

from nbsite.shared_conf import * # noqa: F403

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Keeping this for other side effects

Returns the object or a clone with the options applied
"""
if isinstance(options, str):
from ..util.parser import OptsSpec

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is not related to this change but #6781

@Azaya89 Azaya89 self-assigned this Feb 26, 2026
@codecov

codecov Bot commented Feb 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.70588% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.22%. Comparing base (699da76) to head (76a7f7a).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
holoviews/plotting/plotly/dash.py 0.00% 3 Missing ⚠️
holoviews/tests/test_all.py 85.71% 3 Missing ⚠️
holoviews/util/__init__.py 25.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6802      +/-   ##
==========================================
+ Coverage   89.21%   89.22%   +0.01%     
==========================================
  Files         334      335       +1     
  Lines       72362    72379      +17     
==========================================
+ Hits        64559    64583      +24     
+ Misses       7803     7796       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

from ..core.util.dependencies import _is_installed
from ..util.warnings import deprecated
from . import * # noqa (All Elements need to support comparison)
from . import (

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Remove the elements not used here. This modules is deprecated, so no need to add more them.

@@ -9,12 +9,12 @@

# Holoviews imports
import holoviews as hv

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Changed this back to absolute import. Not entirely sure if it was intended to be run directly at some point

@hoxbro
hoxbro marked this pull request as ready for review February 27, 2026 10:33
@hoxbro
hoxbro requested a review from Azaya89 February 27, 2026 10:33
Comment thread holoviews/operation/__init__.py
Comment thread holoviews/__init__.py Outdated
Dimension.type_formatters[date] = '%Y-%m-%d'
# First one is for Pandas <3 and second for Pandas 3+
Dimension.type_formatters['pandas._libs.tslibs.timestamps.Timestamp'] = "%Y-%m-%d %H:%M:%S"
Dimension.type_formatters['pandas.Timestamp'] = "%Y-%m-%d %H:%M:%S"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not related to changes here, but Pandas 3 upgrade.

@hoxbro
hoxbro force-pushed the refactor_no_star_with_all branch from 76369dc to 2f21b35 Compare March 2, 2026 08:30
@hoxbro
hoxbro merged commit 4fa8c8d into main Mar 4, 2026
17 checks passed
@hoxbro
hoxbro deleted the refactor_no_star_with_all branch March 4, 2026 08:48
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jun 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants