-
Notifications
You must be signed in to change notification settings - Fork 10
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
ENH: Remove lua bindings #37
base: main
Are you sure you want to change the base?
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.
LGTM, thanks a ton @RuhiRG
<< "Fatal Error: The file does not exist or you gave the wrong path.\n"; | ||
// Throw exception? | ||
return *yCloud; | ||
return yCloud; |
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.
This should actually throw. Not for this PR though :)
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.
The test failures might be because the lua
bindings need some work.
0693ba0
to
4c7f9f8
Compare
Co-authored-by: Rohit Goswami <rog32@hi.is>
Since it seems to break on musllinux... https://github.com/d-SEAMS/PydSEAMSlib/actions/runs/10565220956/job/29269283061?pr=21#step:4:1135 In file included from ../subprojects/seams-core/src/backward.cpp:40: ../subprojects/seams-core/src/include/external/backward.hpp:261:10: fatal error: execinfo.h: No such file or directory 261 | #include <execinfo.h> | ^~~~~~~~~~~~
This reverts commit fc84473.
In keeping with #36. Originally this PR was meant to only change the function return types. However, as noted below, this breaks the CI.
So it was later further expanded to completely remove the
lua
bindings, since they are incompatible with the new by value return policies of the functions.Closes #35 as well.