SciML / SciML/PDESystemLibrary.jl

QA: ExplicitImports no_implicit_imports tracked-broken (24 implicit names from symbolic/solver umbrellas)

Open
#63 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Julia
Stars
26
Forks
6
Avg merge
4h 21m
Merged PRs (30d)
11

Description

The QA group (test/qa/qa.jl) now runs ExplicitImports.jl's six checks via run_qa(...; explicit_imports = true) (SciMLTesting v1.6 form). Five pass; check_no_implicit_imports is currently marked ei_broken = (:no_implicit_imports,) (records Broken, auto-flags an Unexpected Pass once resolved).

PDESystemLibrary relies on 24 implicit imports:

using DomainSets: DomainSets, .., Interval, domain
using Interpolations: Interpolations, Gridded, Linear, Periodic, extrapolate, interpolate
using Markdown: Markdown
using ModelingToolkit: ModelingToolkit, @named, @parameters, @variables, Differential, PDESystem, limit
using OrdinaryDiffEq: OrdinaryDiffEq
using OrdinaryDiffEqSDIRK: OrdinaryDiffEqSDIRK, ODEProblem, TRBDF2, solve
using Random: Random

Why this is kept broken rather than mechanically fixed:

  • The names come from heavy re-exporting umbrellas. ExplicitImports reports the owner package, which is frequently not the package the module usings: ../Interval/domain are owned by IntervalSets (re-exported via DomainSets); @variables/Differential/limit by Symbolics; @named/@parameters/PDESystem by ModelingToolkitBase; ODEProblem by SciMLBase; solve by CommonSolve. A using <owner>: name rewrite would pin those internal owner packages, which reshuffle names across releases — exactly the fragile mass-refactor the SciML QA guidance says to avoid.
  • Two of the flagged names are not actually imports at all: limit is a local function defined in lib/brusselator.jl, and domain is a local parameter/variable in lib/general_linear_system.jl and lib/linear_convection.jl. ExplicitImports flags them conservatively, so a blanket explicit-import rewrite would be incorrect.

Resolution options (future work):

  • Make the genuinely-imported names explicit against the umbrella that re-exports them (e.g. using ModelingToolkit: @variables, Differential, PDESystem, @named, @parameters), once it is confirmed stable across the supported ModelingToolkit/DomainSets/OrdinaryDiffEq compat ranges.
  • Or add the local-definition shadows (limit, domain) to the no_implicit_imports ignore list and resolve the rest.

When resolved, drop :no_implicit_imports from ei_broken in test/qa/qa.jl.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with test/qa/qa.jl and the run_qa(...; explicit_imports = true) configuration, then inspect the listed imports and the local definitions in lib/brusselator.jl, lib/general_linear_system.jl, and lib/linear_convection.jl. Confirm a stable resolution across the supported compat ranges, either by making genuine imports explicit or ignoring local-definition shadows; done means check_no_implicit_imports passes and :no_implicit_imports is removed from ei_broken.

Written by the indexing model from the issue text.

Assessment

Tech stack
julia
Domain
testing-qa, tooling
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.