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 doubled arguments generated in bosl2_generator.py #24

Closed
ssuchter opened this issue Apr 20, 2023 · 7 comments
Closed

Fix doubled arguments generated in bosl2_generator.py #24

ssuchter opened this issue Apr 20, 2023 · 7 comments

Comments

@ssuchter
Copy link

ssuchter commented Apr 20, 2023

The raw output of bosl2_generator.py generates some incorrect output. For example, at the moment, this code is produced in bosl2/distributors.py:

def __init__(self, path=None, n=None, spacing=None, sp=None, dist=None, rotate_children=None, dist=None, closed=None, p=None, **kwargs):
       super().__init__("path_copies", {"path" : path, "n" : n, "spacing" : spacing, "sp" : sp, "dist" : dist, "rotate_children" : rotate_children, "dist" : dist, "closed" : closed, "p" : p, **kwargs})

The argument dist=None is listed twice in the __init__ method declaration and in the call to super().__init__. This is a bug.

@jeff-dh
Copy link
Owner

jeff-dh commented Apr 20, 2023

The issue is, that this seems to be "legal" in OpenSCAD but not in Python. The OpenSCAD code really contains these double parameters.
For now I just deleted the second occurrence of the parameter. This works fine, but it actually changes the signature of the corresponding function. Up until now I couldn't come up with a better solution.

If that's the solution, we could take care of it in

I did not had a closer look at it: why are there double parameters in openscad at all? I assume for convenience, but I did not had a closer look at it, what's the semantic behind it?

@jeff-dh
Copy link
Owner

jeff-dh commented Apr 20, 2023

Btw: this is not bosl2_generator.py but openscad_extensions_generator.py

@ssuchter
Copy link
Author

I think it's non-sensical to have double parameters in OpenSCAD, I just don't think there's any validation. I tested this code:

module draw_cube(a, b, a) {
    cube([a, b, a]);
}

draw_cube(1, 2, 3);

It produced a rectangular prism with sides 1, 2, and 1. So while the later duplicate arguments can be specified, they cannot actually be read inside the function. (As I'd kind of expect...)

Since we want solid2 to be robust to whatever upstream BOSL2 feeds it, I think the right thing to do is to change the code generation to match the manual fixes you've had to do.

| Btw: this is not bosl2_generator.py but openscad_extensions_generator.py

Understood, I was just focusing the issue report on the entry point rather than the root cause. Thanks, though.

@jeff-dh
Copy link
Owner

jeff-dh commented Apr 21, 2023

Nonsense.....sorry

@jeff-dh
Copy link
Owner

jeff-dh commented Apr 21, 2023

@jeff-dh
Copy link
Owner

jeff-dh commented Apr 21, 2023

I added a line to the genrator(s) to remove duplicates from the parameter list.

so the genrator(s) do the formerly manual tweaking now.

So I think we can close this issue, right?

@jeff-dh jeff-dh closed this as completed Apr 21, 2023
@ssuchter
Copy link
Author

Yeah, I also had a change that is functionally equivalent to your change, I can abandon mine.

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

No branches or pull requests

2 participants