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

Fix init_printing #371

Merged
merged 7 commits into from
Sep 27, 2021
Merged

Fix init_printing #371

merged 7 commits into from
Sep 27, 2021

Conversation

eendebakpt
Copy link
Contributor

The execution if init_printing failed because in the symengine initialization the symengine.lib import was deleted.

@@ -60,7 +60,6 @@ def lambdify(args, exprs, **kwargs):


# To not expose internals
del lib.symengine_wrapper
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On my system it is needed, since lib.symengine_wrapper is not imported anywhere. I can replace it with

try:
   del lib.symengine_wrapper
except NameError:
   pass

Copy link
Member

Choose a reason for hiding this comment

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

With the latest change, can you try removing this?

Copy link
Contributor Author

@eendebakpt eendebakpt Sep 26, 2021

Choose a reason for hiding this comment

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

@isuruf Back to the old behaviour. Works locally, CI fails but it seems unrelated to this PR.

I understand the desire to hide the internals of symengine in the python interface, but removing the lib.xxx is a bit drastic I think. A better way might be to rename to something that makes it clear it is internal, e.g. internal_lib.xxxx and let the responsibility to use it or not be with the user.

Copy link
Member

Choose a reason for hiding this comment

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

The user can still do import symengine.lib.symengine_wrapper. They have to explicitly do that instead of having access to it when they do import symengine.

symengine/printing.py Outdated Show resolved Hide resolved
eendebakpt and others added 3 commits September 25, 2021 20:16
Co-authored-by: Isuru Fernando <isuruf@gmail.com>
@isuruf isuruf merged commit 29c4264 into symengine:master Sep 27, 2021
@isuruf
Copy link
Member

isuruf commented Sep 27, 2021

Thanks

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.

2 participants