rigetti
fix: make NaN == NaN inside Expression
This is the easiest way to resolve #316 in a way that doesn't break the API (the other way being to use OrderedFloat).
Fixes #316.
Hm. Not sure how to feel about this one. Part of me wants something like Expression::Nan, or Expression::Invalid(DivisionByZero)).
In discussion with @erichulburd today, it seems like the best approach is to take this method, but make it available under a different name rather than PartialEq, and have the round_trip test use that. This should resolve the concern with NaN behaving unexpectedly politely while still allowing us to get the polite behavior.
On second thought, I'm warming to the approach here actually being correct. We claim that Expressions are Eq; that requires promising that equality is an equivalence relation, which is not currently true. It will be true if NaNs compare equal; maybe all that we need to do is document that well and then roll with this change. Thoughts? cc. @erichulburd, @Shadow53, @kalzoo
PR Preview Action v1.6.0 :---: Preview removed because the pull request was closed. 2025-01-14 21:33 UTC
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@Shadow53 I hope you don't mind, I rebased this branch, updated the proptest seed, and added the documentation I mentioned. If this looks good to folks, I think this is ready to merge (subject to CI).
On second thought, I'm warming to the approach here actually being correct. We claim that
Expressions areEq; that requires promising that equality is an equivalence relation, which is not currently true. It will be true if NaNs compare equal; maybe all that we need to do is document that well and then roll with this change. Thoughts? cc. @erichulburd, @Shadow53, @kalzoo
I don't feel too strongly either way and am happy to defer to you and @Shadow53 on this.
