Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix implicit napari dependency #38

Open
jni opened this issue Oct 2, 2024 · 10 comments
Open

Fix implicit napari dependency #38

jni opened this issue Oct 2, 2024 · 10 comments

Comments

@jni
Copy link
Member

jni commented Oct 2, 2024

In #19 we picked up an undeclared dependency on napari:

from napari.utils.naming import CallerFrame

(thanks @tlambert03 for noticing.)

We're only using a small utility from napari, so we could either vendor it or pull it out into its own package...

@psobolewskiPhD
Copy link
Member

psobolewskiPhD commented Oct 2, 2024

Is there a drawback to a napari plugin depending on napari?
I'd say it's almost expected? What would this package provide outside napari that isn't already existing?

@jni
Copy link
Member Author

jni commented Oct 2, 2024

I think there's a drawback to circular dependencies, and there's certainly drawbacks to tight coupling — where you can't change things in napari because of napari-console and vice versa.

@tlambert03
Copy link
Contributor

In this case it would cause a circular dependency (since napari itself also depends on napari console). It's not a typical plugin, and another option is just to undo that (consume it again) and acknowledge the fact that napari depends on it.

@jni
Copy link
Member Author

jni commented Oct 2, 2024

That is an option but not one I like! Again, it's not like we are using actual napari functionality here, just a stack inspection utility that we happened to implement in napari first.

@Czaki
Copy link
Contributor

Czaki commented Oct 2, 2024

In the past, it crashed our constraints compilation. I think that the problem may be fixed.

@jni
Copy link
Member Author

jni commented Oct 2, 2024

it crashed our constraints compilation

"it"?

@Czaki
Copy link
Contributor

Czaki commented Oct 2, 2024

Having napari as dependency of napari dependency:

napari/napari-svg#34
napari/napari#5904

@jni
Copy link
Member Author

jni commented Oct 2, 2024

I think in general though it's not good practice and probably a code smell to have cycles in a dependency graph? (Even if the tools can work around it.)

@tlambert03
Copy link
Contributor

tlambert03 commented Oct 2, 2024

Not sure if it was clear, but I meant the other way around. Since napari console is in naparis deps, maybe just make it part of napari again, (vendored) until you're willing to remove it from the direct deps. What purpose does it serve as an independent thing that only works with napari?

@jni
Copy link
Member Author

jni commented Oct 2, 2024

The goal is to make it a proper plugin — get rid of napari's dependency on napari-console altogether (though we would distribute it as part of the "essentials" in the bundle and napari[all]). (See also napari/napari#4740 😉) Then sure, this can just depend on napari. But yeah moving it in to (hopefully immediately 😅) pull it out does not sound appealing.

So I think that's the answer then — removing the dependency breaks the cycle.

In the meantime, things are basically working so I think this can be ignored?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants