JuliaPy / PythonCall.jl

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

allow user to configure opt_handle_signals #333

Closed brian-dellabetta closed 11 months ago

brian-dellabetta commented 1 year ago

Hi @cjdoris , this PR is to allow a user to configure what is passed to julia --handle-signals=<yes,no>. I have just updated it to be in line with all the other CONFIG params, defaulting to no to retain backwards compatibility.

Following discussion thread starting here, I have found that this is much more robust and clean way to allow for multi-threaded julia code than my previous workaround.

Related to #219 Related to #298 Related to #330

codecov[bot] commented 1 year ago

Codecov Report

Merging #333 (1421a06) into main (e374e25) will decrease coverage by 0.01%. The diff coverage is 100.00%.

:exclamation: Current head 1421a06 differs from pull request most recent head ebf8919. Consider uploading reports for the commit ebf8919 to get more accurate results

@@            Coverage Diff             @@
##             main     #333      +/-   ##
==========================================
- Coverage   41.50%   41.50%   -0.01%     
==========================================
  Files          76       76              
  Lines        4652     4653       +1     
==========================================
  Hits         1931     1931              
- Misses       2721     2722       +1     
Impacted Files Coverage Δ
pysrc/juliacall/__init__.py 71.22% <100.00%> (ø)

... and 1 file with indirect coverage changes

cjdoris commented 1 year ago

Thank you. Could you also update the FAQ.md please with better advice for multithreading?

brian-dellabetta commented 1 year ago

@cjdoris sure thing, just pushed and referenced the discussion thread.

brian-dellabetta commented 1 year ago

@cjdoris I also replied to corresponding issue here. I think users would want signal handling on by default, should we default to yes? It's a behavioral change from previous versions, but almost always wanted

cjdoris commented 11 months ago

Thanks! Please open a separate issue/PR to change the default - AFAIR making it yes by default has undesirable side-effects on Python.