search

adriangb / di

Pythonic dependency injection
https://www.adriangb.com/di/
MIT License
301 stars 13 forks source link

Can we make graphlib2 an optional dependency? #117

Open jriddy opened 12 months ago

jriddy commented 12 months ago

Since graphlib2 is a drop-in replacement for the stldib, can we make it an optional extra? I'm looking to package this for Fedora and avoiding another transitive dep might make it easier.

I'd be happy to submit a PR to do this if you're interested.

adriangb commented 12 months ago

There’d be a pretty large performance hit. And while graphlib2 is a drop in replacement for the standard library the other way around is not necessarily true. Would you use the python backport for < 3.9 or only make it optional for >3.9?

I’d be happy to look at a PR and go from there

jriddy commented 12 months ago

I’ll take a look. Do you have a benchmark or stress test I could run it against? I haven’t yet dived too deep on the code

On Sat, Nov 4, 2023 at 14:26 Adrian Garcia Badaracco < @.***> wrote:

There’d be a pretty large performance hit. And while graphlib2 is a drop in replacement for the standard library the other way around is not necessarily true. Would you use the python backport for < 3.9 or only make it optional for >3.9?

I’d be happy to look at a PR and go from there

— Reply to this email directly, view it on GitHub https://github.com/adriangb/di/issues/117#issuecomment-1793514468, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH5UH4JVPPY6U5DQ4JW37TYC2CGPAVCNFSM6AAAAAA65XKXU6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJTGUYTINBWHA . You are receiving this because you authored the thread.Message ID: @.***>

adriangb commented 12 months ago

They’ll be two sources or performance regressions: rust vs python and copying a prepared graph. The standard library doesn’t let you do the latter, you have to re prepare it. The graphlib2 repo has benchmarks.