Skip to content
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

Improve error message #72

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Improve error message #72

wants to merge 3 commits into from

Conversation

songjhaha
Copy link
Member

@songjhaha songjhaha commented Sep 29, 2022

Relate issue: #71

we can't redirect julia error before we call init_jl() succeeded, a simple solution is to log error message and print it in julia, like:

In [2]: np.init_jl()
   Resolving package versions...
SystemError: opening file "/home/songjhaha/github/jnumpy-repo/jnumpy/JNumPyEnv/Manifest.toml": Permission denied
Stacktrace:
  [1] systemerror(p::String, errno::Int32; extrainfo::Nothing)
    @ Base ./error.jl:174
  [2] #systemerror#68
    @ ./error.jl:173 [inlined]
...
---------------------------------------------------------------------------
JuliaError                                Traceback (most recent call last)
~/github/jnumpy-repo/jnumpy/init.py in init_jl(experimental_fast_init)
    199         try:
--> 200             exec_julia("import TyPython", use_gil=False)
    201         except JuliaError:
...

During handling of the above exception, another exception occurred:

JuliaError                                Traceback (most recent call last)
<ipython-input-2-922bdac64e73> in <module>
----> 1 np.init_jl()

~/github/jnumpy-repo/jnumpy/init.py in init_jl(experimental_fast_init)
    219                     use_gil=False,
    220                 )
--> 221                 raise JuliaError("failed to setup julia environment, check the julia error message above.")
    222         try:
    223             exec_julia(

JuliaError: failed to setup julia environment, check the julia error message above.

I think pyjulia(PyCall) have considered this situation: https://github.com/JuliaPy/pyjulia/blob/56a739126d3fed0d24e548069ff216fea78fbe1b/src/julia/core.py#L587

edit: the solution in pyjulia with lib.jl_typeof_str(lib.jl_exception_occurred()) could only get message like SystemError, so maybe it's not enough for debug.

@thautwarm

@codecov-commenter
Copy link

Codecov Report

Merging #72 (fa7a2b4) into main (89f1c6f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #72   +/-   ##
=======================================
  Coverage   60.97%   60.97%           
=======================================
  Files          11       11           
  Lines         920      920           
=======================================
  Hits          561      561           
  Misses        359      359           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@thautwarm thautwarm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first argument of rethrow is the root cause. Use rethrow().

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

Successfully merging this pull request may close these issues.

3 participants