SciML / SciML/FastAlmostBandedMatrices.jl

QA: tracked-broken Aqua ambiguities + ExplicitImports no_implicit_imports

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

Nobody has claimed this yet.

Dominant language
Julia
Stars
9
Forks
4
Avg merge
5h 54m
Merged PRs (30d)
11

Description

Tracking issue for the two QA findings that are marked known-broken in test/qa/qa.jl after the move to the SciMLTesting run_qa v1.6 form with ExplicitImports enabled. Both are @test_broken placeholders so the QA lane stays green; they should be fixed and the markers dropped.

1. Aqua ambiguities (aqua_broken = (:ambiguities,))

Aqua.test_all reports 19 method ambiguities, all in this package's own methods:

  • ldiv!(::QR{T, <:AlmostBandedMatrix, ...}, ::AbstractVecOrMat{T}) (src ldiv! loop, line ~509) vs ArrayLayouts/LinearAlgebra ldiv!(::Factorization, ::Triangular...) overloads (17 ambiguities).
  • __arguments(::AlmostBandedMatrix, ...) overloads (lines ~532/536/537) ambiguous among themselves (2 ambiguities).

These predate this PR; they were silently suppressed by the prior Aqua.test_all(...; ambiguities = false). The v1.6 conversion preserves the suppression as a visible, counted @test_broken rather than a silent disable. Fix: disambiguate the ldiv!/__arguments method tables (e.g. add the specific intersection methods Aqua suggests, or narrow the signatures).

2. ExplicitImports no_implicit_imports (ei_broken = (:no_implicit_imports,))

31 names are used via implicit imports through the package's
using ArrayInterface, ArrayLayouts, BandedMatrices, ConcreteStructs, LazyArrays, LinearAlgebra, MatrixFactorizations, Reexport plus @reexport using BandedMatrices. The full list (from ExplicitImports.print_explicit_imports):

  • ArrayLayouts: LayoutMatrix, Lmul, TriangularLayout
  • BandedMatrices: BandedMatrix, bandwidth, bandwidths, brand
  • ConcreteStructs: @concrete
  • LazyArrays: LazyArray, Ldiv, Mul
  • LinearAlgebra: LowerTriangular, NoPivot, UnitLowerTriangular, UnitUpperTriangular, UpperTriangular, diagind, lmul!, lu, qr, rank, triu!
  • (plus the bare module names ArrayInterface, ArrayLayouts, BandedMatrices, ConcreteStructs, LazyArrays, LinearAlgebra, MatrixFactorizations, Reexport, @reexport)

This is deferred (not done in the conversion PR) because it is a large, mechanical-but-broad using X: a, b, c refactor across 8 dependencies, and the @reexport using BandedMatrices re-export must be preserved. Should be done as its own focused PR.

Opened as part of the SciMLTesting run_qa v1.6 + ExplicitImports rollout.

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 run the QA checks to reproduce the tracked failures. Review the ldiv! loop near src line 509 and the __arguments methods near lines 532, 536, and 537, then audit the listed imports across the package. Done means the Aqua and ExplicitImports markers can be removed while preserving the @reexport using BandedMatrices behavior.

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
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.