Add support for sympy.sign in .printing.pycode #14010
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
|
Member
Author
|
Nice catch! (I'm surprised that copysign isn't returning NAN here). But isn't that a "bug" in |
Member
|
It's probably because it just copies the sign bit and NaN has a sign bit. |
Member
Author
|
I tracked down the issue. The problem does not lie in One possible work-around would be to define the method |
Member
Author
|
@ylemkimon since the |
ylemkimon
approved these changes
Feb 9, 2018
Member
ylemkimon
left a comment
ylemkimon
left a comment
There was a problem hiding this comment.
I meant a discrepancy between SymPy's sign returning NaN for NaN and Python's copysign returning +-1 for NaN, but it's an implementation issue, not a code printer should handle. LGTM 😄
aaron-skydio
added a commit
to aaron-skydio/sympy
that referenced
this pull request
Mar 11, 2025
This is half issue, half PR - I'm not 100% sure exactly how this should be correctly implemented, and I'm not necessarily going to have time to test this further, but the fix is small enough and the code change demonstrates the issue better than an issue would. As I understand it, `sympy.sign` is defined to return 0 for 0. Other code printers also implement it this way, see for example sympy#14010. Rust's `signum` returns 1 or -1 for 0 (depending on the sign of the 0, for floats with signed zeroes), which isn't correct.
aaron-skydio
pushed a commit
to aaron-skydio/sympy
that referenced
this pull request
Mar 11, 2025
This is half issue, half PR - I'm not 100% sure exactly how this should be correctly implemented, and I'm not necessarily going to have time to test this further, but the fix is small enough and the code change demonstrates the issue better than an issue would. As I understand it, `sympy.sign` is defined to return 0 for 0. Other code printers also implement it this way, see for example sympy#14010. Rust's `signum` returns 1 or -1 for 0 (depending on the sign of the 0, for floats with signed zeroes), which isn't correct.
aaron-skydio
pushed a commit
to aaron-skydio/sympy
that referenced
this pull request
Mar 11, 2025
This is half issue, half PR - I'm not 100% sure exactly how this should be correctly implemented, and I'm not necessarily going to have time to test this further, but the fix is small enough and the code change demonstrates the issue better than an issue would. As I understand it, `sympy.sign` is defined to return 0 for 0. Other code printers also implement it this way, see for example sympy#14010. Rust's `signum` returns 1 or -1 for 0 (depending on the sign of the 0, for floats with signed zeroes), which isn't correct.
aaron-skydio
added a commit
to aaron-skydio/sympy
that referenced
this pull request
Mar 11, 2025
This is half issue, half PR - I'm not 100% sure exactly how this should be correctly implemented, and I'm not necessarily going to have time to test this further, but the fix is small enough and the code change demonstrates the issue better than an issue would. As I understand it, `sympy.sign` is defined to return 0 for 0. Other code printers also implement it this way, see for example sympy#14010. Rust's `signum` returns 1 or -1 for 0 (depending on the sign of the 0, for floats with signed zeroes), which isn't correct.
firatbezir
pushed a commit
to firatbezir/sympy
that referenced
this pull request
Mar 21, 2025
This is half issue, half PR - I'm not 100% sure exactly how this should be correctly implemented, and I'm not necessarily going to have time to test this further, but the fix is small enough and the code change demonstrates the issue better than an issue would. As I understand it, `sympy.sign` is defined to return 0 for 0. Other code printers also implement it this way, see for example sympy#14010. Rust's `signum` returns 1 or -1 for 0 (depending on the sign of the 0, for floats with signed zeroes), which isn't correct.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Python doesn't have a
math.signfunction, it does have acopysignfunction though, however, just wrapping copysign would not give the right answer forsign(0)(hence the use of the ternary operator).