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

Using __slots__ #242

Closed
Midnighter opened this issue Aug 13, 2018 · 5 comments · Fixed by #243
Closed

Using __slots__ #242

Midnighter opened this issue Aug 13, 2018 · 5 comments · Fixed by #243

Comments

@Midnighter
Copy link
Contributor

Midnighter commented Aug 13, 2018

I'm working on a package that makes heavy use of sympy and we would like symengine to be a drop-in replacement. Since in our context there might be anywhere from a few to hundreds of thousands of variables, I'd like to build the variables using __slots__.

That approach works perfectly when inheriting from sympy.Dummy but unfortunately not when subclassing from symengine.Symbol. (I'm building on top of symengine 0.3 so I couldn't find Dummy there.)

Would it be hard for you to redefine your Python class interface to use __slots__?

@isuruf
Copy link
Member

isuruf commented Aug 13, 2018

I'm not familiar with __slots__ can you give a small example that doesn't work?

@Midnighter
Copy link
Contributor Author

Midnighter commented Aug 13, 2018

Sure, please take a look at the linked notebook. Basically, classes with slots are more compact and deterministic. However, if you don't define __slots__ in your class, Python automatically defines the __dict__ attribute for you. If any class in your parent hierarchy does this, it is pointless to use slots in child classes because __dict__ is already defined. The linked StackOverflow answer in my above comment has a very extensive description and I recommend reading it when you have the time. Please note that in the notebook I also care about the __weakref__ attribute because my code needs it but that actually doesn't really relate to this issue.

@isuruf
Copy link
Member

isuruf commented Aug 13, 2018

Note that all symengine classes deriving from Basic has only one attribute. The pointer to the C++ object.

Cython extension classes like Basic has a mechanism similar to __slots__. So, if you do pi.__dict__ it will throw an error. (pi is an object of type Constant which is a Cython extension class)

All the classes in symengine used to be Cython extension classes and then some were changed to be regular Python classes so that __new__ could be used for simulating SymPy behaviour of Add(1, 2) returning an Integer class.

Symbol was changed to a regular Python class at 83bd7ae and I'm not sure why. There are two options,

  1. Change Symbol to be a Cython extension class
  2. Define __slots__ = () in class Symbol

Both should work. I'm not sure which is the best. I guess we can go with 1 and switch to option 2 if that doesn't work.

Can you send a PR with the above change and a test case?

@Midnighter
Copy link
Contributor Author

Option 2. I can easily do, while for option 1. I feel that I know too little about symengine. However, if that is your preferred way I can make a suggestion and you can review it.

@isuruf
Copy link
Member

isuruf commented Aug 13, 2018

Option 1 is basically changing class Symbol(Expr) to cdef class Symbol(Expr)

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 a pull request may close this issue.

2 participants