Skip to content

Cleans implicit imports + Add ExplicitImports quality assurance testsuite#239

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

Cleans implicit imports + Add ExplicitImports quality assurance testsuite#239
navidcy wants to merge 30 commits into
mainfrom
ncc/clean-imports

Conversation

@navidcy
Copy link
Copy Markdown
Member

@navidcy navidcy commented May 14, 2026

Closes #229

@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2026

@navidcy navidcy requested a review from giordano May 14, 2026 02:38
@navidcy navidcy marked this pull request as ready for review May 14, 2026 02:38
@navidcy
Copy link
Copy Markdown
Member Author

navidcy commented May 14, 2026

@giordano I tried to follow Oceananigans practices. The tests pass here now. Could you have a look? There is a chance I misunderstood something.

I am a bit confused because I was thinking that the end goal would be that there is no import ... whatsoever.

@navidcy
Copy link
Copy Markdown
Member Author

navidcy commented May 14, 2026

8497c2a tried to have all extension of methods explicit.

@navidcy navidcy added the clean up 🧹 technical debt is real people label May 14, 2026
@giordano
Copy link
Copy Markdown
Member

For what is worth, the way I go when I want to implement ExplicitImports is to run

using ExplicitImports, MyPackage
print_explicit_imports(MyPackage)

or

using ExplicitImports, MyPackage
print_explicit_imports(MyPackage.Submodule)

if MyPackage is very large, and working on a submodule-basis is more efficient (that's what I've been doing in Oceananigans). This is the recommended approach in the docs and is quite nice because it shows you exactly what's needed to change, and how to change it. Then I use some judgement (for example it always suggests using AnotherPackage: AnotherPackage, even if not actually needed, I think as a precaution), but I have some good guidance.

@navidcy
Copy link
Copy Markdown
Member Author

navidcy commented May 14, 2026

I've done it... I believe this is ready to merge?
I haven't cleaned all extensions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clean up 🧹 technical debt is real people package 📦 testing 🧪 trust but verify

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cleaning imports

2 participants