Adopt module-path privacy convention (ADR-012)#648
Open
ecomodeller wants to merge 3 commits into
Open
Conversation
Function names inside private modules drop the leading underscore; the module path already signals "internal." Cross-cutting helpers that lived in public modules (utils.py, metrics.py) move into a new modelskill/_utils.py or into comparison/_utils.py so they don't accidentally become public surface. Pyright's reportPrivateUsage warnings disappear; Ruff PLC2701 now enforces the rule in CI. See adr/012-public-private-api-convention.md for the policy and rationale.
- Soften ADR-012 + CLAUDE.md to describe what's actually enforced: PLC2701 is the hard rule; underscores on function names are an optional convention. - Split _utils.RESERVED_NAMES into RESERVED_COORD_NAMES (coord-name collisions) and RESERVED_COMPARER_VAR_NAMES (+Observation slot), expressing the superset relationship in code instead of by shadowed in-function constants. - Extract linear_regression from _utils.py into its own single-purpose _regression.py, leaving _utils.py as the name-handling helpers module.
"utils" is non-informative — the module's actual concern is reserved coordinate/variable names and name-or-index resolution. Naming it _names.py says what's inside and what isn't. ADR-012 updated to reflect the rename and justify the choice; CLAUDE.md example updated.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Establishes a written convention distinguishing public API from internal API, and aligns the codebase with it.
Policy (full text in ADR-012): Privacy is set by the module path, not the function name. A name is private if any segment of its import path starts with
_. Functions inside private modules use plain names — the path already carries the signal. This matches the scikit-learn convention (sklearn.utils._param_validation.validate_params).Enforced by Ruff
PLC2701(import-private-name), which now runs in CI and reports zero violations insrc/. Tests have a per-file ignore.Concrete changes:
_timeseries.py,_misc.py,_base.py,_point.py,_track.py,_vertical.py,comparison/_utils.py) drop their leading_.modelskill/_utils.pyforget_name,get_idx,RESERVED_NAMES,linear_regression;parse_metricmoves tocomparison/_utils.py.modelskill/timeseries/__init__.pyno longer re-exports the five_parse_*_inputhelpers in__all__(they were simultaneously public-by-__all__and private-by-name). Importers inmodel/*.pyandobs.pynow use the deep private paths.modelskill.utilskeeps its currently-reachable public surface (rename_coords_xr,make_unique_index, etc.) — no breaking changes to non-underscored import paths.Test plan
ruff check src/reports 0 PLC2701 violationssorted(n for n in dir(modelskill) if not n.startswith('_'))matchesmainfrom modelskill.utils import rename_coords_xrstill worksfrom modelskill.timeseries import _parse_track_inputcorrectly raisesImportError(intentionally removed)