-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add support for negation to f64 #34
Conversation
This PR does require updating https://github.com/saulshanabrook/egg-smol first, I think? |
Yeah I am keeping an upstream fork currently to pull in the visualization work which isn't merged yet (egraphs-good/egglog#147). I just merged in the recent commits, including yours, to that branch. If you update the pinned commit to If you want to add a small test to |
Thanks! I've made these changes - but I'm hitting a persistent error ( |
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 left a comment about removing some lines from the test file to help it pass...
I am not sure about the pytest import error! Could you post the full traceback from the command? What OS are you running with as well?
Also if you wouldn't mind adding an entry to the changelog that would be great!
Done, thanks for the pointer!
Also done!
I fear this may have just been a transient bug with my Python virtualenv setup? Activating the env in a fresh terminal allows |
Looks like the only remaining failure is with Python 3.8. Rust is failing to compile egglog, due to using some unstable rust feature:
It looks like this feature was added to rust 1.70.0 rust-lang/rust#93050 (comment) I am not sure why the other python version builds work fine? EDIT: It looks like the 3.9 test used a newer runner image:
than the 3.8 run:
I will try re-running the 3.8 manually. |
Thank you for adding this and being the first external contributor! I just made a release so this will be in v0.5.1. |
Now that upstream
egglog
supportsneg
forf64
(egraphs-good/egglog#168), this PR adds support to thef64
sort in these bindings.Closes #32