-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Can we make graphlib2 an optional dependency? #117
Comments
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 |
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
<#117 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAH5UH4JVPPY6U5DQ4JW37TYC2CGPAVCNFSM6AAAAAA65XKXU6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJTGUYTINBWHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
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. |
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.
The text was updated successfully, but these errors were encountered: