search

JuliaPy / PythonCall.jl

Python and Julia in harmony.
https://juliapy.github.io/PythonCall.jl/stable/
MIT License
722 stars 61 forks source link

Dont use __init__ #300

Open rafaqz opened 1 year ago

rafaqz commented 1 year ago

Given recent precompilation concerns it would be better not to use and recommend using __init__ here.

Instead pynew() could have a runtime value check, and do the import automatically on the first call?

We could pass it a string for the module name, or a closure.

cjdoris commented 1 year ago

Nice idea.

Would be good to investigate if adding such a check has any performance penalty, since it will be done for every python operation.

cjdoris commented 1 year ago

Of course if any package does anything with PythonCall (even indirectly) in its __init__ then the effect is undone. But we can at least recommend against it.

rafaqz commented 1 year ago

Was this actually implemented?

cjdoris commented 1 year ago

No, I guess I hit the Close button by mistake!

pedromxavier commented 1 year ago

@cjdoris is there any benchmark suite of yours? I'd like to test a few a ideas for such lazy-loading mechanism.

cjdoris commented 1 year ago

Just https://github.com/cjdoris/PythonCall.jl/blob/main/BENCHMARKS.md which may be out of date. Might be nice to formalise them.

github-actions[bot] commented 10 months ago

This issue has been marked as stale because it has been open for 30 days with no activity. If the issue is still relevant then please leave a comment, or else it will be closed in 7 days.

github-actions[bot] commented 10 months ago

This issue has been closed because it has been stale for 7 days. You can re-open it if it is still relevant.

LilithHafner commented 9 months ago

IMO this is still relevant, it should be re-opened and added to a milestone so that it is not automatically re-closed as stale.

rafaqz commented 9 months ago

Auto closing issues after 30 days doesnt really make sense, Im not going to keep pinging all my githib issues

cjdoris commented 9 months ago

Don't worry, pinging every 30 days is not the intention - it's to let actual stale issues die and prompt something to happen with inactive issues, even if the something is simply to add a label that prevents it from being marked stale again, which I have just done.

That said, this was marked stale because of inactivity. I'm unlikely to work on this myself so if you want anything to happen you need to make it happen.

rafaqz commented 9 months ago

Marking stale due to inactivity is a questionable practice. Issues that are not fixed don't just go away.

There is no chance I will work on this, it was basically a "heads up" issue for future design.

cjdoris commented 9 months ago

Like I said, issues which are actual issues I can label so they stick around, like I did with this one.

The bot is to close the issues that aren't actually issues - such as because it actually got resolved or the user didn't provide a way to reproduce the issue.

rafaqz commented 9 months ago

Issues can be closed from PRs using e.g. closes #x . Or manually if they dont matter.

But auto closing will end up with discussions like this - there is a reason almost no other packages do it. But your call of course.