-
Notifications
You must be signed in to change notification settings - Fork 22
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
Make gap error handler throw an error #144
Conversation
Codecov Report
@@ Coverage Diff @@
## master #144 +/- ##
==========================================
+ Coverage 56.4% 56.44% +0.03%
==========================================
Files 44 44
Lines 2599 2599
==========================================
+ Hits 1466 1467 +1
+ Misses 1133 1132 -1
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice :-). Though now I wonder about more complicated situations: GAP calling Julia calling GAP etc. and then an error is thrown.
@@ -25,7 +25,7 @@ read_sysinfo_gap = function(dir::String) | |||
end | |||
|
|||
function error_handler(message) | |||
print(message) | |||
error("Error thrown by GAP") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this perhaps include the message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will check how to achieve this. It seemed to me that the message gets printed by GAP anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see: I thought this is just a mater of printing message
. But now looking at how the "error callback" is called, I realized that it actually gets no arguments -- and this signature here is incorrect.
The complicated solution should work this way as well: GAP catches Julia error and goes into the GAP error, and Julia catches the GAP error and goes into Julia error. I think this works in arbitrary recursion depth. |
I am not so sure that this code would work right with arbitrary interleaved calls; indeed, I even think that it's not yet enough to support some GAP code from within Julia without any mutual recursion at all:. Because we are actually setting the I am sorry for being a bit fuzzy, but I don't have time right now to work out exact examples and give more details; the whole "error handling" in GAP is a bit of a mess right now for various reasons, and also extremely complicated; I've been working on untangling it a bit for the past year, but its still far from me how to fix it in GAP alone, and I am far from being clear on how it should work when adding Julia as another player. (And once we also want to integrate it with Singular and Polymake, it'll be even more challenging; but that's actually a major point for using Julia as the central glue for all these systems, because this way, at least we only need to solve this very complicated domain (error handling between two interfaced systems) once for every pair, and not for all pairs. |
0fd11e0
to
fb52449
Compare
@fingolfin I rebased this commit and fixed the function signature. For now, I gave up on the stream business. We could for example create a GAP stream, and set I would prefer to be able to give |
Resolves #114