Skip to content

Adding support for is_integer attribute #2686

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

Merged
merged 1 commit into from
May 6, 2024

Conversation

anutosh491
Copy link
Collaborator

This Pr is trying to support the following case (and is also required for the gruntz algorithm)

from lpython import s
from sympy import Symbol

def main0():
    a: S = Symbol('x')
    b: S = S(10)
    x: bool = a.is_integer
    y: bool = b.is_integer
    print(x)
    print(y)
    
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine examples/expr2.py 
False
True

@anutosh491
Copy link
Collaborator Author

Ready !

Copy link
Collaborator

@ubaidsk ubaidsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a failure at the CI. Apart from that this looks good.

@ubaidsk ubaidsk marked this pull request as draft May 5, 2024 09:05
@anutosh491
Copy link
Collaborator Author

There is a failure at the CI. Apart from that this looks good.

Hmm, pretty weird, I wasn't expecting any test to fail. You can quickly check the results on sympy live (https://live.sympy.org/), A WebAssembly-powered Python kernel backed by Pyodide
image

@anutosh491
Copy link
Collaborator Author

@certik could you maybe try having a look ?

@ubaidsk
Copy link
Collaborator

ubaidsk commented May 5, 2024

@anutosh491 The following diff fixes the issue for me:

diff --git a/src/runtime/lpython/lpython.py b/src/runtime/lpython/lpython.py
index 9f23b02e9..a8b88ac6f 100644
--- a/src/runtime/lpython/lpython.py
+++ b/src/runtime/lpython/lpython.py
@@ -15,6 +15,10 @@ __slots__ = ["i8", "i16", "i32", "i64", "u8", "u16", "u32", "u64", "f32", "f64",
 
 # data-types
 
+def get_sympy_S(x):
+    from sympy import S
+    return S(x)
+
 type_to_convert_func = {
     "i1": bool,
     "i8": int,
@@ -34,7 +38,7 @@ type_to_convert_func = {
     "Callable": lambda x: x,
     "Allocatable": lambda x: x,
     "Pointer": lambda x: x,
-    "S": lambda x: x,
+    "S": get_sympy_S,
 }
 
 class Type:

@anutosh491
Copy link
Collaborator Author

anutosh491 commented May 6, 2024

Hey thanks Ubaid, I think this might fix the issue for us.

@anutosh491 anutosh491 marked this pull request as ready for review May 6, 2024 10:10
@ubaidsk
Copy link
Collaborator

ubaidsk commented May 6, 2024

We should clean the commit history or squash merge this.

Copy link
Collaborator

@ubaidsk ubaidsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good. Thanks for this! Amazing work!

@ubaidsk ubaidsk enabled auto-merge May 6, 2024 10:36
@ubaidsk ubaidsk merged commit 41f8d49 into lcompilers:main May 6, 2024
@anutosh491 anutosh491 deleted the is_integer branch May 6, 2024 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants