search

SolidCode / SolidPython

A python frontend for solid modelling that compiles to OpenSCAD
1.11k stars 173 forks source link

Add support for numpy parameters #83

Closed justbuchanan closed 6 years ago

etjones commented 6 years ago

This should be fine, Justin. I haven't used Numpy for any SolidPython work. Looks like this PR resolves one particular chokepoint using Numpy. Have you found any others? I'm a little hesitant to just import a package as big as Numpy deep in a conditional someplace. It might be that your change is all we need to be Numpy-friendly (in which case I'll just merge it) but I'm curious if there's a slightly bigger change we should make for general Numpy compatibility. Any thoughts on this?

justbuchanan commented 6 years ago

This is the only chokepoint I've found when using numpy with solidpython. With these changes, I've managed to render a few different models to scad successfully.

I understand the hesitation with importing numpy deep in a conditional as I have it now. Another option would be moving the import to the top of the file and just continuing if it's not available:

try:
  import numpy
except ImportError:
  pass

Do you think that's a bit better?

etjones commented 6 years ago

No, I think what you’ve done is the right thing. And if the conditional for a numpy data type evaluated as true, you’ll almost certainly have numpy in your environment, which is good. There is still a code smell about it, but I prefer your original solution to doing a likely-unnecessary large import on startup. I’ll merge and close this. Thanks for thinking it through with me

On Nov 27, 2017, at 12:24 PM, Justin Buchanan notifications@github.com wrote:

This is the only chokepoint I've found when using numpy with solidpython. With these changes, I've managed to render a few different models to scad successfully.

I understand the hesitation with importing numpy deep in a conditional as I have it now. Another option would be moving the import to the top of the file and just continuing if it's not available:

try: import numpy except ImportError: pass Do you think that's a bit better?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.