Skip to content

Drop the _allocation suffix from the public functions #55

Description

@fpedd

Four public names carry an _allocation suffix that repeats what the package
name and the argument already say:

om.validate_allocation(pool)
om.plot_allocation(pool)
om.io.save_allocation(pool, path)
om.io.load_allocation(path)

The suffix is not descriptive. It exists because validate.py and
visualize.py are modules, so importing from them in __init__.py binds the
module as a package attribute and takes the short name. om.validate today is
a module, and om.validate(pool) raises TypeError: 'module' object is not callable.

_allocate.py already solves this: the module is private, so allocate is
free to be the function.

Proposal

Module New name
omnimalloc/validate.py omnimalloc/_validate.py
omnimalloc/visualize.py omnimalloc/_visualize.py
omnimalloc/io.py unchanged
Now Proposed
om.validate_allocation(entity) om.validate(entity)
om.plot_allocation(entity, path) om.plot(entity, path)
om.io.save_allocation(entity, path) om.io.save(entity, path)
om.io.load_allocation(path) om.io.load(path)

io.py stays public

The first draft of this issue proposed _io.py re-exported as io. That is
wrong on both halves.

It is unnecessary: the suffix problem comes from __init__.py importing the
module, and __init__.py never imported .io#49 removed save_allocation
and load_allocation from the top level. Nothing shadows the short names
there.

It is also breaking. from omnimalloc.io import load resolves a real
submodule, not a package attribute, so from . import _io as io would raise
ModuleNotFoundError for every caller using that spelling, including
benchmark/sources/minimalloc.py.

Instead __init__.py imports the module, so om.io resolves after a plain
import omnimalloc, and all three spellings work:

om.io.save(pool, path)                  # via the package attribute
from omnimalloc.io import save, load    # via the submodule
import omnimalloc.io                    # directly

Resulting top level, nine names (#49 already established the other eight;
this issue renames two of them and adds io):

__all__ = [
    "Allocation", "Memory", "Pool", "System",
    "__version__", "allocate", "io", "plot", "validate",
]

The rename is not purely mechanical

Three call sites take a validate: bool parameter and call the validator in
the body, so a plain find-and-replace makes the parameter shadow the function
and the call site invokes a bool:

  • _allocate.allocate
  • benchmark._benchmark_result
  • scripts/generate_readme_assets._solve

All three alias the import (from ._validate import validate as _validate).
ty catches the miss as call-non-callable, which is how the third one
surfaced — it is not reachable from the test suite.

Two more spots a mechanical sweep misses:

  • LANE_CAVEAT and PANEL_CAVEAT in _visualize.py embed
    validate_allocation() in a string that is rendered as the figure
    suptitle. They now read omnimalloc.validate(), qualified, because a bare
    validate() in a figure caption says nothing.
  • CLAUDE.md names all four functions and is untracked, so it appears in no
    diff and no git grep.

Scope

Occurrence counts on main (88f8303):

Name Files Occurrences
validate_allocation 33 171
plot_allocation 12 45
save_allocation 2 21
load_allocation 3 14

One module import needs redirecting: tests/unit/test_visualize.py does
from omnimalloc import visualize and monkeypatches through it; it now
monkeypatches by dotted path instead.

Follow-on cleanups

The rename establishes one rule — the module is an implementation detail, the
package __init__ is the API
— and the same rule applies in four more places.
Each is a separate commit.

  1. __all__ in every package. Shrink the top-level namespace to eight names #49 gave the top level an __all__; the
    eight subpackage __init__ files still use from .x import Y as Y. Two
    spellings for one job. Also surfaced two gaps where omnimalloc.benchmark
    did not mirror omnimalloc.benchmark.sources: SkewedSource,
    TwoPlusTwoSource, SIZE_DISTRIBUTIONS, sample_sizes.

  2. analysis/ speaks one dialect. _conflicts.py and _pressure.py were
    private, clock.py and linearize.py were not. clock.py was the only
    module the package did not re-export, so seven modules across four packages
    reached past __init__ for time_components and uniform_dim. Both are
    now exported; the modules become _clock.py and _linearize.py.

    This surfaced a latent import cycle. primitives/pool.py bound
    analysis._pressure at module scope for Pool.pressure — the one upward
    reference in the layering — while analysis imports primitives. Nothing
    broke only because _conflicts happened to sort first in the __init__ and
    pulled primitives in before _clock ran. Putting _clock first closed
    analysis -> _clock -> primitives -> pool -> _pressure -> _clock on a
    half-initialized module. The upward reference is now deferred into
    Pool.pressure, so no import order can close it.

  3. _benchmark is the same suffix. run_benchmark/plot_benchmark/
    save_benchmark become om.benchmark.run/.plot/.save, and
    benchmark/benchmark.py and results/visualize.py become _benchmark.py
    and _visualize.py. Timer joins the exports — four scripts imported it
    from omnimalloc.benchmark.timer, the same reach-past-__init__ that
    clock.py invited.

  4. benchmark no longer imports the root package. benchmark/benchmark.py
    did from omnimalloc import allocate, validate_allocation, the one import
    that made a subpackage depend on the package __init__. Folded into the
    rename commit since the names change there anyway.

Considered and dropped

  • Normalizing absolute vs relative imports. The 135/168 split is not drift:
    ruff runs select = ["ALL"], so TID252 is active with
    ban-relative-imports = "parents". The rule is already enforced — . for
    same-package siblings, absolute where .. would be needed.
  • common -> _common. common/__init__.py is empty, so nothing leaks and
    the change is cosmetic. It would touch ~30 files and conflict with all five
    open PRs. Worth doing when the queue drains, not now.

Notes

No deprecation aliases. The project is pre-1.0 and there are no external
consumers to carry.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions