Skip to content

Import public API from its owning module, not a re-exporter - #4880

Merged
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:agent/qa-explicitimports
Aug 7, 2026
Merged

Import public API from its owning module, not a re-exporter#4880
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:agent/qa-explicitimports

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member

Please ignore until reviewed by @ChrisRackauckas.

Problem

On unmodified master (32a955a), the QA lane's ExplicitImports checks fail. Two of them are about provenance: ModelingToolkit reaches ten bindings through a package that merely forwards them, rather than through the module that declares them.

########## explicit_imports_via_owners
Module `ModelingToolkit` has explicit imports of names from modules other than their owner:
- `@add_kwonly` has owner `SciMLBase` but it was imported from `DiffEqBase`
- `Operator` has owner `SymbolicUtils` but it was imported from `Symbolics`
- `Term` has owner `SymbolicUtils` but it was imported from `Symbolics`
- `_isone` has owner `SymbolicUtils` but it was imported from `Symbolics`
- `_iszero` has owner `SymbolicUtils` but it was imported from `Symbolics`
- `getname` has owner `SymbolicIndexingInterface` but it was imported from `Symbolics`
- `maketerm` has owner `TermInterface` but it was imported from `SymbolicUtils`
- `metadata` has owner `TermInterface` but it was imported from `SymbolicUtils`

########## qualified_accesses_via_owners
Module `ModelingToolkit` has qualified accesses to names via modules other than their owner:
- `NullParameters` has owner `SciMLBase` but it was accessed from `DiffEqBase`
- `value` has owner `SciMLBase` but it was accessed from `DiffEqBase`

Fixing those exposed four more in the ModelingToolkit.StructuralTransformations submodule (ExplicitImports reports the first failing module only): topological_sort (owner Graphs), value and var_from_nested_derivative (owner Symbolics), and schedule (owner Base), all imported from ModelingToolkitBase.

Change

Each name is imported from its owner. schedule had no use site in StructuralTransformations (only sys.schedule and get_schedule appear), so it is dropped rather than re-pointed. TermInterface becomes a direct dependency; it was already in the manifest via SymbolicUtils.

Verification

All 13 relocated bindings are === to the ones previously used, so this is a pure import-site change:

@add_kwonly                   true
Operator                      true
Term                          true
_iszero                       true
_isone                        true
getname                       true
maketerm                      true
metadata                      true
NullParameters                true
value                         true
topological_sort              true
MTKBase.value                 true
var_from_nested_derivative    true
ALL IDENTICAL = true

Both checks now pass (run against this branch on Julia 1.11):

########## explicit_imports_via_owners
OK

########## qualified_accesses_via_owners
OK

Six of the moved names are public at their owner but not at the re-exporter, so the two remaining *_are_public checks also improve, from 57 to 53 explicit-import findings and from 91 to 89 qualified-access findings. Those two checks are not closed by this PR.

Functional checks run locally against this branch:

  • test/semilinearodeproblem.jl (the file containing the DiffEqBase.value -> SciMLBase.value change): Test Summary: Astrochem model | 61 Pass 6 Broken 67 Total 5m57.7s
  • Ad-hoc smoke: Lorenz mtkcompile + ODEProblem + solve(Rodas5P()), index-3 pendulum mtkcompile (exercises StructuralTransformations), and linearization_function/linearize all pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_019ywCW8vbGoc9X3dUbmXyme

`ExplicitImports.check_all_explicit_imports_via_owners` and
`check_all_qualified_accesses_via_owners` both failed on `ModelingToolkit`
because ten bindings were reached through a package that merely forwards them:

  - `@add_kwonly`, `NullParameters`, `value` via `DiffEqBase` (owner `SciMLBase`)
  - `Operator`, `Term`, `_iszero`, `_isone` via `Symbolics` (owner `SymbolicUtils`)
  - `getname` via `Symbolics` (owner `SymbolicIndexingInterface`)
  - `maketerm`, `metadata` via `SymbolicUtils` (owner `TermInterface`)
  - `topological_sort`, `value`, `var_from_nested_derivative`, `schedule` via
    `ModelingToolkitBase` in the `StructuralTransformations` submodule
    (owners `Graphs`, `Symbolics`, `Symbolics`, `Base`)

Each name is now imported from the module that declares it. Every relocated
binding was checked to be `===` to the one previously used, so this is a pure
import-site change with no behaviour difference. `schedule` was unused and is
dropped rather than re-pointed.

Four of the moved names (`@add_kwonly`, `NullParameters`, `value`, `getname`)
plus `maketerm`/`metadata` are `public` at their owner but not at the
re-exporter, so this also drops the `*_are_public` findings from 57/91 to
53/89.

`TermInterface` becomes a direct dependency; it was already in the manifest
through `SymbolicUtils`.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants