-
Notifications
You must be signed in to change notification settings - Fork 6
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
fix #22 #23
base: master
Are you sure you want to change the base?
fix #22 #23
Conversation
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.
Thank you very much for the fixes. Iirc this also fixed a second issue that I think has been requested to specify the heap size.
I guess it will break with older GAP versions but that is maybe not so important as GAP users don't tend to linger on old versions for so long.
I'll need to update the CI at some point and also bump the supported CPython versions but LGTM otherwise.
return CloseOutput(&output); | ||
} | ||
""" | ||
UInt GAP_OpenOutputStream(Obj stream) |
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.
Why is this a string? Is this a Cython trick I forgot?
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.
sorry which string?
oh shoot i just reverted the #18 memory fix because the tests failed! Would you like me to put it back? specify gap's heap size in the gap initializer like
where gap_memory is the integer valued heap size in megabytes |
I made MakeStringWithLen non-static, and changed the type of the first variable from char to const char.
I replaced MarkBag with GAP_MarkBag from libgap.
I passed dummy variables to OpenOutputStream and CloseOutput.