Skip to content
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

Exposed get_base and get_exp functions #1980

Closed

Conversation

anutosh491
Copy link
Contributor

No description provided.

@anutosh491
Copy link
Contributor Author

@isuruf Not sure why some tests fail here. Everything passes for me locally!
Am I missing some flag while building the project through cmake ?

symengine/cwrapper.cpp Outdated Show resolved Hide resolved
Copy link
Member

@isuruf isuruf left a comment

Choose a reason for hiding this comment

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

This can already be achieved by using basic_get_args right?

@anutosh491
Copy link
Contributor Author

This can already be achieved by using basic_get_args right?

Yes, I missed this. I've made the change.

@isuruf
Copy link
Member

isuruf commented Oct 3, 2023

No, I mean a user could use that method already in the C wrapper and I don't see why we'd need another method to do something similar.

@anutosh491
Copy link
Contributor Author

anutosh491 commented Oct 3, 2023

Okay for some context, Ondrej and I would be adding support for some methods through the Cwrapper for addressing this TODO lcompilers/lpython#2332
We are trying to port some functionalities like the ones we could have through SymPy using Lpython (like expr.base and expr.exp) so that we could essentially support some modules like series through Lpython. Yeah I agree we could use the .args method but this is just so that we could introduce some consistency with SymPy.

@anutosh491
Copy link
Contributor Author

Also could you educate me regarding the failing tests ?
When I run ctests locally everything works as required .

@certik
Copy link
Contributor

certik commented Oct 3, 2023

I think we should be designing the Symbolic API to be consistent, and I think we have done a lot of this work in SymEngine already. In particular, I think it's cleaner to access the arguments using .args, so I would just use that.

Regarding the failing tests, not sure what is causing it, you would need to debug it locally.

@certik certik marked this pull request as draft October 3, 2023 07:51
@certik
Copy link
Contributor

certik commented Oct 3, 2023

I think we can close this one. If you think we need it, you can reopen it.

@certik certik closed this Oct 3, 2023
@anutosh491
Copy link
Contributor Author

Sure if we can do without creating these then let's try it out. I just thought of introducing some consistency with sympy but I think we can do without it !

@certik
Copy link
Contributor

certik commented Oct 3, 2023

LPython adds the consistency with SymPy. In SymEngine I think we should strive for a minimal C++ and C API, without duplication.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants