qa: import names through owning modules in ModelingToolkitBase - #4989
Merged
ChrisRackauckas merged 1 commit intoAug 18, 2026
Merged
Conversation
ModelingToolkitBase still used the blanket-`using` import style that the umbrella package was already migrated away from, so its ExplicitImports checks errored: 200 implicit imports, 42 stale explicit imports, and a number of names reached through a re-exporter rather than their owner. Import each name from the module that defines it, drop the stale ones, and add the curated non-public and re-export lists that `test/qa/qa.jl` already carries for ModelingToolkit. `TermInterface`, `StaticArraysCore` and `IntervalSets` become direct dependencies since they own names already in use; all three were already transitive and are MIT. `ADTypes` moves to test-only, its source use having been dead. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
ChrisRackauckas
marked this pull request as ready for review
August 18, 2026 08:00
ChrisRackauckas-Claude
pushed a commit
to ChrisRackauckas-Claude/ModelingToolkit.jl
that referenced
this pull request
Aug 18, 2026
SciML#4989 trimmed `hessian_sparsity`, `MultithreadedForm`, `SerialForm`, `CTarget` and `derivative` from ModelingToolkitBase's Symbolics import list, as ExplicitImports reported them unused in `src`. They were still reached as `ModelingToolkitBase.<name>` from tests, and none is exported by Symbolics, so the re-export does not cover them. That broke InterfaceII and Extended. Reference them through Symbolics, which owns them. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
ChrisRackauckas
added a commit
that referenced
this pull request
Aug 18, 2026
#4989 trimmed `hessian_sparsity`, `MultithreadedForm`, `SerialForm`, `CTarget` and `derivative` from ModelingToolkitBase's Symbolics import list, as ExplicitImports reported them unused in `src`. They were still reached as `ModelingToolkitBase.<name>` from tests, and none is exported by Symbolics, so the re-export does not cover them. That broke InterfaceII and Extended. Reference them through Symbolics, which owns them. Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com>
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.
What and why
lib/ModelingToolkitBasestill used the blanket-usingimport style thatsrc/ModelingToolkit.jlwas already migrated away from, so all six of its ExplicitImports checks errored, and its re-export check had no approved list. This is the debt explicitly deferred in #4934 ("The full strict QA still has separate baseline JET, ExplicitImports, and unapproved-reexport debt. This PR does not suppress or allowlist those failures.").What was wrong, concretely:
using SciMLBase,using LinearAlgebra,using Graphs, …)Chain/Postwalk/Prewalk/Fixpoint,BuildTargets/JuliaTarget,gradient/hessian/derivative/sparsehessian,Base: RefValue,ADTypes: AutoForwardDiff, …)_EvalModuleOpt*submodules implicitly importingRuntimeGeneratedFunctions@data-generated submodules (MissingGuessValue,StructuralHint) being unanalyzablepublic_reexportsreporting all 145 externally-owned public names, becauserun_qawas called withoutreexports_allowThis imports each name from the module that defines it, deletes the stale ones, and adds the curated lists that
test/qa/qa.jlalready carries for ModelingToolkit —NONPUBLIC_EXPLICIT_IMPORTS(27),NONPUBLIC_QUALIFIED_ACCESSES(96),REEXPORTED_API(145), andallow_unanalyzablefor the two Moshi modules. No check is disabled and no wildcard is used; every entry names something genuinely owned elsewhere.Dependency changes
TermInterface,StaticArraysCoreandIntervalSetsbecome direct deps ofModelingToolkitBase, since they own names it already uses (maketerm/metadata,SVector/MVector,endpoints) that were previously reached through re-exporters. All three were already transitive dependencies, and all three are MIT — same as ModelingToolkitBase — so this adds no new licensing surface.ADTypesmoves from[deps]to test-only: its source use was dead, and leaving it in[deps]would fail Aqua's stale-dependency check.Verification
GROUP=QA(ModelingToolkitBase)GROUP=InterfaceI(ModelingToolkitBase)InterfaceI | 1599 pass, 5 broken, 1604 total— passedGROUP=QA(umbrella ModelingToolkit)QA | 52 pass, 52 total— passed; exercises@import_mtkbase, which pulls in every MTKBase nameusing ModelingToolkit,@mtkcompile,ODEProblem→u0 = [1.0]runic --checkoverlib/,src/,test/typosThe QA group goes from 13 pass / 2 fail / 6 error to 20 pass / 1 fail / 0 error.
This does NOT make the QA job green — the JET lane still fails
The one remaining failure is
JET-test failed,═════ 263 possible errors found ═════. It is unchanged by this PR: the findings were extracted before and after, gensym counters normalised, and the diff is empty (263 lines either way).Of the 263, 249 are
local variable ##And#NNN#f#1 may be undefined/##Call#NNN#...— JET's typo mode misreading the short-circuit guards thatMoshi.Match.@matchlowers to, the same class already tracked in #4958, JuliaLang/julia#62745 and aviatesk/JET.jl#858. The remaining 14 areis not definedfindings that look genuine and are separate bugs (convert_system×2,get_parameter_dependencies,ModelingtoolkitizeParametersNotSupportedError, field accesses.v/.sz/.s/.new/.expr, localsnewbuf×3,dsti,buffer).test/qa/qa.jlalready setsjet = falsefor ModelingToolkit with a comment citing those three issues. Applying the same treatment tolib/ModelingToolkitBase/test/qa/aqua.jlwould be a one-line change — but that is disabling a failing check, so I have deliberately not done it. Whether to mirror that precedent here is a maintainer call, and the 14 real findings probably deserve their own issue either way.Scope
Deliberately kept to import hygiene and the QA lists — no behavior changes. Split out of #4983, where these failures surfaced, so that a mechanical sweep does not ride along with a behavior change.
Not verified
GROUP=InterfaceII, the docs build, downstream and downgrade lanes, and non-Linux or non-1.12 Julia versions were not run locally.Please ignore until reviewed by @ChrisRackauckas.