Skip to content

Quality assurance tests via ExplicitImports + Aqua#144

Open
navidcy wants to merge 16 commits into
mainfrom
ncc/clean-imports
Open

Quality assurance tests via ExplicitImports + Aqua#144
navidcy wants to merge 16 commits into
mainfrom
ncc/clean-imports

Conversation

@navidcy
Copy link
Copy Markdown
Member

@navidcy navidcy commented May 24, 2026

Add quality assurance tests via ExplicitImports

Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread test/test_quality_assurance.jl
@navidcy
Copy link
Copy Markdown
Member Author

navidcy commented May 24, 2026

Question: The type piracy errors for fields and prognostic_fields would be healed if

fields(::Nothing) = NamedTuple()
prognostic_fields(::Nothing) = NamedTuple()

are defined in Oceananigans rather than here?

@navidcy
Copy link
Copy Markdown
Member Author

navidcy commented May 24, 2026

@giordano if you can have a look at the Aqua errors and help me with proposed solutions it'd be great!
I'm not sure what to do with the free_drift_u/free_drift_v ambiguities for example.

@navidcy navidcy changed the title Quality assurance tests via ExplicitImports Quality assurance tests via ExplicitImports + Aqua May 24, 2026
@giordano
Copy link
Copy Markdown

@giordano if you can have a look at the Aqua errors and help me with proposed solutions it'd be great! I'm not sure what to do with the free_drift_u/free_drift_v ambiguities for example.

In general there isn't a recipe for how to handle ambiguities, that's the point of being ambiguous.

In this case the problem is

const TISB = StressBalanceFreeDrift{<:Any, <:SemiImplicitStress}
const BISB = StressBalanceFreeDrift{<:SemiImplicitStress, <:Any}
@inline function free_drift_u(i, j, k, grid, f::TISB, clock, fields)
@inline function free_drift_u(i, j, k, grid, f::BISB, clock, fields)
TISB constrains the first parameter of StressBalanceFreeDrift, BISB constrains the last one. What to do where you have the mixed case? 🤷 That's entirely up to who wrote the code.

@navidcy
Copy link
Copy Markdown
Member Author

navidcy commented May 24, 2026

Yes. Perhaps @simone-silvestri is better to comment.
Or I can define a method for the mixed case that returns and error and we see if it ever gets triggered by the tests?

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