Repository navigation
A module is never hidden behind a function of its own name, and every public module says what it publishes - #973
Merged
jmrplens merged 1 commit intoOct 21, 2026
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Important Review skippedToo many files! This PR contains 429 files, which is 329 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configuration
⛔ Files ignored due to path filters: 1
📒 Files selected for processing: 429
You can disable this status message by setting the
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jmrplens
added this pull request to stack #984
October 17, 2026 17:11
jmrplens
changed the title
github-actions
Bot
added
documentation
github-actions
Bot
added
area: signals
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## fix/acronyms-and-spelled-out-words #973 +/- ##
=====================================================================
Coverage ? 96.86%
=====================================================================
Files ? 444
Lines ? 89874
Branches ? 0
=====================================================================
Hits ? 87058
Misses ? 2816
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Numerical conformance
1893/1893 checks pass across 111 domains and 523 standards (227 normative designations, 125 further published sources). Used in the tables below is how much of that clause's published tolerance the deviation consumes: 100 % sits exactly on the limit, 5 % uses a twentieth of the allowance, and a dash means the clause states no two-sided tolerance for the quantity, so there is no budget to spend. It is reported and never used to decide a verdict, which is settled at full precision before any rounding. Renamed (3)
Closest to their published limit (top 5) The rows with the least room left, so the ones a change is most likely to push over.
|
| Python Version | Tests | Failures | Coverage | Status |
|---|---|---|---|---|
| macos-latest-3.12 | 28427 | 0 | 96.9% |  Passed |
| macos-latest-3.13 | 28427 | 0 | 96.9% | Passed |
| macos-latest-3.14 | 28427 | 0 | 96.9% | Passed |
| ubuntu-latest-3.12 | 28427 | 0 | 96.9% | Passed |
| ubuntu-latest-3.13 | 28427 | 0 | 96.9% | Passed |
| ubuntu-latest-3.14 | 28427 | 0 | 96.9% | Passed |
| windows-latest-3.12 | 28427 | 0 | 96.9% | Passed |
| windows-latest-3.13 | 28427 | 0 | 96.9% | Passed |
| windows-latest-3.14 | 28427 | 0 | 96.9% | Passed |
Full report at this commit: docs/CONFORMANCE.md · docs/conformance.json · full CI artifacts
… public module says what it publishes Thirty-one modules shared their name with a function their package publishes. `signals/correlation.py` sat beside `signals.correlation()`, so the package attribute `signals.correlation` was the function: `import phonometry.signals.correlation as m` bound the function, `signals.correlation.time_delay` raised `AttributeError`, and only a `from` import of the file path reached the module, while the API reference had a page titled with that address. The same held for `room.impulse_response`, `fluids.air`, the eight sound power modules of `emission` and twenty more. And 119 public modules had no `__all__`, so a constant one of them kept for itself looked as public as its functions; some of those constants had reached guides and scripts through the full module path, where nothing could rename them without breaking a reader.
jmrplens
force-pushed
the
fix/no-module-behind-a-function
branch
from
October 20, 2026 01:45
de8da16 to
0bc8fbe
Compare
 [Medium impact] Renames internal modules to follow naming conventions. The PR appears safe to merge, with two non-blocking fixes recommended for the new API check. FindingsReviews (1) · Last reviewed commit: "A module is never hidden behind a functi..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+649
to
+652
| if isinstance(node, ast.Import): | ||
| for alias in node.names: | ||
| if alias.asname == leaf: | ||
| return f"import {alias.name} as {leaf}" |
There was a problem hiding this comment.
_binding_other_than_submodule reports import pkg.probe as probe as hiding probe, even though that statement binds the module itself. A contributor using this valid import form gets a false error from the new API check.
Exclude imports whose full name is f"{package}.{leaf}", and add this case to _BINDINGS.
Owner
Author
Comment on lines
+687
to
+690
| for defined in defined_names(module): | ||
| if defined.startswith("_") or defined in listed: | ||
| continue | ||
| obj = getattr(module, defined, None) |
There was a problem hiding this comment.
defined_names collects names from both sides of every if and every try branch. _unlisted_problems then reports those names even when the imported module does not contain them. For example, a fallback helper defined only under except ImportError is reported when the dependency imports successfully.
Skip names absent from vars(module) before checking whether they are listed, and test branches that define different names.
Suggested change
| for defined in defined_names(module): | |
| if defined.startswith("_") or defined in listed: | |
| continue | |
| obj = getattr(module, defined, None) | |
| for defined in defined_names(module): | |
| if defined.startswith("_") or defined in listed or defined not in vars(module): | |
| continue | |
| obj = getattr(module, defined, None) |
Owner
Author
There was a problem hiding this comment.
I am leaving this as it is on purpose. A name bound in any branch of an if or a try is a name the module binds on some install or interpreter, so it takes a leading underscore or goes in __all__ whichever branch ran on the machine doing the check. Skipping names absent from vars(module) would make the verdict depend on what that machine has installed, and a fallback helper defined for the module's own use is exactly what the rule asks to make private (_helper).
There was a problem hiding this comment.
That’s a fair clarification. This is intentionally a conservative source-level check: it enforces the naming/publication rule for every branch that may bind a name, rather than making the result depend on which optional dependencies happen to be installed when the check runs. In that design, a fallback-only helper is still an internal module name that must be underscored (or explicitly published), so my suggested vars(module) guard would weaken the portability guarantee. No change is needed.
|
10 tasks done
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
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.
Thirty-one modules shared their name with a function their package publishes.
signals/correlation.pysat besidesignals.correlation(), so the package attributesignals.correlationwas the function:import phonometry.signals.correlation as mbound the function,signals.correlation.time_delayraisedAttributeError, and only afromimport of the file path reached the module, while the API reference had a page titled with that address. The same held forroom.impulse_response,fluids.air, the eight sound power modules ofemissionand twenty more. And 119 public modules had no__all__, so a constant one of them kept for itself looked as public as its functions; some of those constants had reached guides and scripts through the full module path, where nothing could rename them without breaking a reader.What changes
Each of the 31 modules is private now (
signals/_correlation.py), the convention PEP 8 sets for internal interfaces and the one NumPy (numpy._core) and SciPy follow for their implementation modules. The function keeps its name, and every name the package published from the module keeps its address:signals.correlation(...)andsignals.time_delayread as before, andfrom phonometry.signals import time_delayis the import. Every importer moved: the package__init__files, sibling modules,_plotand_report, the tests, the conformance domains, figure, diagram and report scripts,api_taxonomy.py, the errata, the docstrings and the guides.The API reference keeps one page per topic at the URL it had, so no link breaks, but no page claims a module address that does not import any more. A private module's page is titled by its domain and subject (
signals: correlation), opens by naming the package its names are imported from, and names the other package where one of its names is published elsewhere (plot_junction_geometry, drawn beside the vibration junctions, is published bybuilding). The subject is words and never a standard's designation, because the site search weighs a title above the standards a guide declares: titledtonality (ECMA-418-2), the reference page came within 1.6 times of the guides written to ECMA-418-2 on a search for it, where the site check asks for twice. The three ECMA-418-2 pages arepsychoacoustics: tonality (Sottek model),roughness (Sottek model)andfluctuation strength (Sottek model), and their summaries still name the standard. The index lists such a page by its domain, and the guides link to it by that title (signals: correlation) instead of asignals.correlationlabel that reads as a module path.Every public module now declares an
__all__that lists what it defines and its package publishes, and anything else it defines takes a leading underscore.hearing.hearing_protectorsandnoise_control.valvesnow list the constants their packages publish from them. The three NC tables a guide drew by hand are published byroomwith their unit:room.NC_OCTAVE_BANDS_HZ,room.NC_RATINGSandroom.NC_CURVES_DB, each with a curated row. The SII band importances, band centres and vocal efforts are read fromspeech.sii_procedure(...)andspeech.standard_speech_spectra(), the NIPTS frequencies from thehearing.nipts(...)result, the ISO 1683 references from the.valueof themetrology.ISO1683_REFERENCE_VALUESrecords, and the guides now read them there. The coefficients, limits and defaults ofenvironment.assessment.impulsive_sound,vibration.human.multiple_shock,building.prediction.detailed_modeland the rest are private, each applied by the function that reads it or taken as the default of a parameter; two constants nothing read,electroacoustics.piston.J1_FIRST_ZEROandnoise_control.duct_modes.PLANE_WAVE_EIGENVALUE, are deleted. Type aliases,TypeVars andProtocols stayPascalCaseand unlisted, as the rest of the tree has them.Four of the values that went private are criteria no function applies, and the migration guide now says so instead of the opposite: the ECMA-418-2 prominence thresholds of tonality (0.4 tu_HMS, clause 6.3), roughness (0.2 asper, clause 7.2) and fluctuation strength (0.2 vacil_HMS, clause 9.2), and its 0.01 sone_HMS audibility criterion (clause 5.1.9). The guide gives each with its clause and the result field to compare against it. The sound quality guide said the standard attaches no limit to the single tonality value T; clause 6.3 gives two criteria, 0.4 tu_HMS on the specific tonality at a local maximum for a stationary sound and 0.4 tu_HMS on T for a non-stationary one, and the guide now states both and that
tonality_ecma()applies neither.What breaks
The 31 leaf import paths are gone, with no alias:
importlib.import_module("phonometry.signals.correlation")raisesModuleNotFoundError. The names those modules defined and their packages never published go private with their module: the gates and block lengths ofbroadcast.program_loudness, the ECMA-418-2 prominence thresholds, the uncertainty defaults of the tone audibility module, the duct path self-noise floor, and the type aliases of fouremissionmodules (PrecisionSurface,PrecisionArray,PrecisionRoom,MicrophoneShield,Grade,BandType,DeterminationGrade), whoseLiteralis what to annotate with now. The migration guide, in its three copies, has a new section with the 23 leaf paths that shipped in 4.0.0rc1 and what to write instead, and a table of every constant withdrawn from a module that shipped in 3.3.0 or 4.0.0rc1, under its 3.3.0 module and spelling where those differ (DEFAULT_AIR_CONDITIONbecamePUBLISHED_AIR_CONDITIONin rc1), with where its value lives now:metrology.ISO1683_REFERENCE_VALUES["solid"]["velocity_alternative"].valuefor the 50 nm/s free-velocity reference, the 1 m/(N s) reference mobility of EN 15657 that the functions apply, the 25 mm microphone spacing of IEC 61043 Table 2 thatspacing_m=defaults to, and, forreference_soundof the DIN 45692 module, how to build the clause 6 test signal fromsignals.noise_signal. The eight modules newer than rc1 are in the CHANGELOG only, and the CHANGELOG entries written since rc1 now name the package instead of the file or a constant that went private. CONTRIBUTING has a section on the rule and why. The counts at the top of the migration guide do not move: no name the 3.3.0 top level exported changes here.How it was verified
No number moves. The conformance report runs 1893 of 1893 checks and
docs/CONFORMANCE.mdcomes out byte for byte the same; the figures whose generators changed re-render byte-identical;make reportsreproduces every committed fiche; the API reference covers all 3062 public names and its pages keep their URLs (check:redirectsgreen). The clip fingerprints do not move exceptanim_epnl_flyover, which was already stale on the base with the same fingerprint. The full suite passes on Python 3.13 (28358 passed, 69 skipped), every documentation code block runs (7535 blocks over 731 pages), and the site builds and validates (astro checkwith no error or warning,html-validateclean, andcheck:searchgreen for all 306 designations in both languages, the worst margin 2.4 times on ISO 3744).Every value the migration guide now gives was read on the printed page: ECMA-418-2:2025 clause 6.3 (PDF page 34, printed page 24), clause 7.2 (PDF page 46, printed page 36), clause 9.2 (PDF page 61, printed page 51) and clause 5.1.9 (PDF page 23, printed page 13); DIN 45692:2009-08 clause 6 (PDF page 9), whose 920 to 1080 Hz noise at 60 dB built from
signals.noise_signalgives 1.0004 acum throughpsychoacoustics.sharpness_din; UNE-EN ISO 1683:2016 Table 3 and its note b (PDF page 9, printed page 9); UNE-EN 15657:2018 clause 5 for the 1 m/(N s) reference mobility (PDF page 11, printed page 11).room.NC_CURVES_DBis now compared cell by cell with ANSI/ASA S12.2-2019 Table 1, typed from the printed page (PDF page 18, printed page 6); a one-decibel change to any cell fails the test, where before only the NC-30 row and a few cells through the SIL were held.The guard
scripts/check_api_reference.pyholds the modules to both rules, throughmodule_problems, which walks every public module of every domain package (the packages themselves included):__init__, where any statement butfrom . import <module>that binds the module's name hides it, so a module another import reached first is found as well;__all__is reported;__all__and leaves out a name it defines and its package publishes is reported;TypeVar, aProtocol). An exception is keyed by location inUNLISTED_EXEMPTIONSwith its reason, and an entry that no longer applies is reported, so the table cannot go stale. The table is empty: the reference spacing ofemission.intensity_compliancecarries its unit now, and with it its underscore (_REFERENCE_SPACING_M).tests/test_check_api_reference.pypins each rule on fake packages (a module hidden behind a function and one behind a string, a module beside the function its package publishes from another file that nothing imports, the same module imported before the walk, a domain package without__all__, a list that leaves out a published name and an unpublished constant, an alias, aTypeVar, aProtocol, an import and a private module that all stay quiet), the statements of an__init__that do and do not hide a module, the stale exemption, and the AST walk that reads what a module binds; it also runs the rules over the library. The two new fake-package cases and the statement table fail against the guard without these reads. A copy ofsignals/_correlation.pydropped beside it assignals/correlation.pymakes the script exit 1 with the hidden-module message.tests/test_removed_names.pypins the 31 leaf paths as gone, a sample of the withdrawn constants, and the NC tables at their new address.tests/test_api_docs_generator.pyholds every page title to an address that imports, every private page to its domain, and every name on a private page to the package its opening names; it refuses a title with a number in it, which is how a designation would reach one (red on the three(ECMA-418-2)titles), and a title too long for the 70 charactershtml-validateallows the browser title.Red on the base (c719ff6), with this guard copied in (trimmed; the full output is 166 lines):
That is 31 hidden modules, 119 modules without
__all__and 15 names left out of a list. Green on this branch: