napari: import-linter blocking commit

🐛 Bug

I am not able to commit to my branch because the import-linter is failing on code that I didn’t touch. See here:

import-linter............................................................Failed
- hook id: import-linter
- exit code: 1
=============
Import Linter
=============
---------
Contracts
---------
Analyzed 553 files, 2248 dependencies.
--------------------------------------
"Forbid import PyQt and PySide" KEPT
"Block import from qt module in abstract ones" KEPT
"Block import from qt module in abstract ones" KEPT
"Block import from qt to _qt" BROKEN
Contracts: 3 kept, 1 broken.
----------------
Broken contracts
----------------
"Block import from qt to _qt"
-----------------------------
napari._qt is not allowed to import napari.qt:
- napari._qt._tests.test_progress -> napari.qt.progress (l.11)
- napari._qt._tests.test_progress -> napari.qt (l.11)

To Reproduce

Steps to reproduce the behavior:

Expected behavior

Environment

Please copy and paste the information at napari info option in help menubar here:
Any other relevant information:

Additional context

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 24 (16 by maintainers)

Most upvoted comments

So should we3 close this @sofroniewn ?

goanpeca on Jun 1, 2021

FYI: I have been able to commit since #2774 was merged. 😃

ppwadhwa on May 25, 2021

I have now merged #2774, hopefully that should improve the situation, we can see and then decide if we can close this

sofroniewn on May 24, 2021

is this just stylistically a good idea or is it absolutely critical - i.e. code will crash if this is true?

could crash because of circular import.

I already meet something similar (not napari) when crash depends on order of import so when GUI is starting everything works, but when usin nongui parts of code (readers, writers, calculation backend) there exist orders of imports which ends with crash.

But I agree that fast drop could be a good idea to not block development process.

Czaki on May 24, 2021

I add this contract to ensure that noting in napari._qt depends on napari.qt.

is this just stylistically a good idea or is it absolutely critical - i.e. code will crash if this is true?

I’d probably just drop right now (option 1), make sure that people can commit code again, and then we can have a longer discussion if this thing is really critical.

sofroniewn on May 24, 2021