Repository navigation
Trig functions equate radian-based Python functions with degree-based OpenSCAD functions #44
Description
The trig functions defined in https://github.com/jeff-dh/SolidPython/blob/master-2.0.0-beta-dev/solid2/core/object_base/object_base_impl.py are implemented as follows:
import math
sin = lambda x: OpenSCADConstant(f'sin({x})') if isinstance(x, OpenSCADConstant) else math.sin(x)
cos = lambda x: OpenSCADConstant(f'cos({x})') if isinstance(x, OpenSCADConstant) else math.cos(x)
tan = lambda x: OpenSCADConstant(f'tan({x})') if isinstance(x, OpenSCADConstant) else math.tan(x)
asin = lambda x: OpenSCADConstant(f'asin({x})') if isinstance(x, OpenSCADConstant) else math.asin(x)
acos = lambda x: OpenSCADConstant(f'acos({x})') if isinstance(x, OpenSCADConstant) else math.acos(x)
atan = lambda x: OpenSCADConstant(f'atan({x})') if isinstance(x, OpenSCADConstant) else math.atan(x)
However the stdlib math functions take arguments in radians (0-2pi), while the OpenSCAD functions take arguments in degrees (0-360). Therefore you get different results depending on whether the argument is an OpenSCAD constant or something that is processed in Python.
You can test the difference with this function:
def trigtest():
shapes = (
solid2.cube(.5).translateZ(solid2.sin(360)) +
solid2.cube(.5).translateZ(solid2.sin(solid2.OpenSCADConstant("360")))
)
shapes.save_as_scad("trigtest.scad")
OpenSCAD's sin(360) is 0, while Python's math.sin(360) is ~0.96, so you get two cubes, one at the origin and one translated up 0.96 units.
The correction would be to convert the radians before using the Python built-in:
sin = lambda x: OpenSCADConstant(f'sin({x})') if isinstance(x, OpenSCADConstant) else math.sin(math.radians(x))
...
Reacted by jeff
Activity
jeff-dh commentedon Sep 22, 2023
I'll think about it.......
I'm wondering whether it might make more sense to comply with the python standard than to the openscad standard...... or we could remove the "if-else" stuff.... the "if-else-solution" is legacy stuff anyway....
lfagundes commentedon Sep 22, 2023
I suggest having two packages, one for radians, another for degrees. But then, there's another issue: the habit of importing *, which is not very pythonic. I find it very difficult to understand the documentation, because I have no idea what's in the namespace, and once a function is used, I'm not sure where it comes from.
In fact, I even made a PR for this, because I had no idea these functions already exist, because they're scrambled in the namespace. Declaring everything is more work, but enables handling more complexity.
jeff-dh commentedon Sep 23, 2023
see 'import *' is not a recommended practice
Furthermore the sin/cos/... issue is a legacy issue that was not changed up until now for backwards compatibility reasons. The new way of calling them is:
which is unambiguous and solves all the issues I assume (even though the openscad sin and python sin take different arguments, but that's because the underlying libraries (python / openscad) operate that way and I don't know whether I want to change that).
I think about removing them from the "global" solid2 namespace even though it would create backwards compatibility issues.
For now I prefer the solution to remove the trigonometric functions from the solid2 namespace.
jeff-dh commentedon Sep 23, 2023
suggestions and prs are welcome! ;) (there's already one concerning the docs. I'm absolutely fine with restructuring / reworking the docs!)
SubstructureDek commentedon Sep 25, 2023
I'm new to SolidPython but keeping them fully and explicitly separate (by removing them from the global namespace) makes a lot of sense to me and allows you to keep the conventions associated with both contexts.
removed trigonometric functions from solid2 namespace (#44)
jeff-dh commentedon Sep 25, 2023
"fixed" in master-2.... branch.
I'll package it soon into a solidpython 2.1 version. This should also include the "real cylinder fix" and thus would be a (single) backwards incompatible version change.
If there's a need for
sin_rad, cos_rad,...let me know.lfagundes commentedon Sep 30, 2023
I see the convenience of import *, and personally I don't use. Instead, I declare each function being imported. It's more work, but I think it's worth on the long run. This way I also avoid polluting code with a module name.
If a developer wants to use import * for convenience, that's ok. But documentation would benefit a lot from declaring everything, as more people would understand the underlying architecture, and that would give more community consistency. And that's why Python is so strong, it's all about the Zen of Python. Explicit is better than implicit.
jeff-dh commentedon Oct 2, 2023
I see what you're saying, but for me SolidPython is not pure python, it's also OpenSCAD. At least that's -- from my point of view -- the heritage and what -- I think -- makes SolidPython strong(er than other python openscad wrappers). It's possible to almost copy and paste OpenSCAD code into SolidPython with minor modifications (cmp: bosl2_diff) without worrying about anything (imports).
From that point of view this discussion scratches at a paradigm change from the up until now "as close to OpenSCAD as possible"-style (
import *) to a python-style (explicit imports or "subnamespaces").This also touches #37. It might be a question "where you're coming from". The point of view might be different for people with OpenSCAD background and for people without OpenSCAD background. SolidPython can be seen as OpenSCAD on steroids (my primary point of view up until now) or as a "regular" python library....
I would suggest to restructure the readme and as one of the first points discuss the different import styles and their pros and cons and who should use what in which situation. I think about using explicit imports for the examples.....
Btw: I don't have a lot time to spent on SolidPython right now, the progress will be a little bit slower.....
use explicit imports in examples (#44 #45)
jeff-dh commentedon Oct 6, 2023
https://github.com/jeff-dh/SolidPython/wiki/Importing-SolidPython