Skip to content

Exposed get_base and get_exp functions - #1980

Closed
anutosh491 wants to merge 2 commits into
symengine:masterfrom
anutosh491:Expose_get_base_and_exp
Closed

Exposed get_base and get_exp functions#1980
anutosh491 wants to merge 2 commits into
symengine:masterfrom
anutosh491:Expose_get_base_and_exp

Conversation

@anutosh491

Copy link
Copy Markdown
Contributor

No description provided.

@anutosh491

Copy link
Copy Markdown
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 ?

Comment thread symengine/cwrapper.cpp Outdated

@isuruf isuruf left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
Contributor Author

This can already be achieved by using basic_get_args right?

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

@isuruf

isuruf commented Oct 3, 2023

Copy link
Copy Markdown
Member

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

anutosh491 commented Oct 3, 2023

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Contributor Author

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

@certik

certik commented Oct 3, 2023

Copy link
Copy Markdown
Contributor

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

certik commented Oct 3, 2023

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

certik commented Oct 3, 2023

Copy link
Copy Markdown
Contributor

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.

3 participants