A warning too far?

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

A warning too far?

nophead

WARNING: length() parameter could not be converted, in file global.scad, line 104


v = is_undef(len(d)) ? [0, 0, d] : d;

I think this is the standard OpenSCAD idiom to test if a parameter is a vector or a scalar. Is there any other way to do it?


I think it is sufficient that taking the length of a scalar gives undef and using undef other than in is_undef() gives a warning, so you can't do it accidentally.


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

doug.moen
Well, there is also this feature request: https://github.com/openscad/openscad/issues/1584


On Mon, Feb 4, 2019, at 12:23 PM, nop head wrote:

WARNING: length() parameter could not be converted, in file global.scad, line 104


v = is_undef(len(d)) ? [0, 0, d] : d;

I think this is the standard OpenSCAD idiom to test if a parameter is a vector or a scalar. Is there any other way to do it?


I think it is sufficient that taking the length of a scalar gives undef and using undef other than in is_undef() gives a warning, so you can't do it accidentally.

_______________________________________________
OpenSCAD mailing list


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

Parkinbot
In reply to this post by nophead
I don't get this warning. Further, I tend to write:

v = len(d)?d:[0,0,d];

I didn't even know that is_undef() exists.



--
Sent from: http://forum.openscad.org/

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

nophead
is_undef is very new but had to be added because using undef is now a warning. 

If is_list() is added then I could live with this warning, otherwise I can't because it breaks my library. This is a bleeding edge version from today, if you use that you will get the warning. 

The new warnings and errors are very welcome but I update frequently from master and nearly every change means I have to change my code. In this case I don't know any way to implement this without the wall of warnings. As I said before though I don't think this one actually adds anything. Also it mentions length() but it should be len().

I think people will get a big shock when they update because it will give a wall of warnings for typical old code. For example I was relying on zero size cylinders and cubes, etc. Now I have to conditionally remove them.






On Mon, 4 Feb 2019 at 23:11, Parkinbot <[hidden email]> wrote:
I don't get this warning. Further, I tend to write:

v = len(d)?d:[0,0,d];

I didn't even know that is_undef() exists.



--
Sent from: http://forum.openscad.org/

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

JordanBrown
On 2/4/2019 4:19 PM, nop head wrote:
is_undef is very new but had to be added because using undef is now a warning.


Hmm?

You mean you can't say

    radius = (r != undef ) ? r : d/2;

?

I have lots of instances of that pattern, mostly for supplying calculated defaults.  (And note:  while some are defaults for module parameters and so could maybe be handled using the built-in parameter defaulting mechanism, others are for vectors with optional elements.)



_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

MichaelPFrey
In reply to this post by nophead
On 05.02.19 01:19, nop head wrote:
> is_undef is very new but had to be added because using undef is now a
> warning.
>
> If is_list() is added then I could live with this warning, otherwise I
> can't because it breaks my library.

I will look into implementing it ones I am finished with
https://github.com/openscad/openscad/pull/2757


> The new warnings and errors are very welcome but I update frequently
> from master and nearly every change means I have to change my code. In
> this case I don't know any way to implement this without the wall of
> warnings. As I said before though I don't think this one actually adds
> anything.

It is hard to foresee the consequences of warnings.

(which is why I try to not change how openscad behaves, except for
outputing the warning)

is_undef was/is added, when I added the warning for undefined special
variables.

(which is confusing concept in its own rights - a variable can be
undefined or be defined as undefined which is not the same thing when it
comes to warnings. But at the end of the day, it was/is about preventing
typos in variable names)

The warning for undefined parameters is helpful in the context for the
geometry functions or other functions used as parameters for other
functions.

Having to find what caused an undef can be very hard, as previously,
giving undef into a function lead to it silently accept the undef as a
parameter and silently returning undef. Combine that with how openSCAD
functions need to be written and can get messy, when you iterate over a
vector, to create an other vector to create a bar diagramm.

> Also it mentions length() but it should be len().
My bad. I will fix that.
> I think people will get a big shock when they update because it will
> give a wall of warnings for typical old code.
That is a complicated balance act. For newcomers (and new designs), it
will be useful, as a lot of complicated symptoms now create clear
warnings, when you write the code.
> For example I was relying on zero size cylinders and cubes, etc. Now I
> have to conditionally remove them.

or you disable "check the parameter range for builtin modules" in the
advanced tab.

I know that this is a double edged sword.

I knew that this one could be problematic, as I was writing code where
negative cube sizes not leading to geometry was both used for the design
to work and at the same time it was confusing that I wanted 10 x 10, but
only got 5x5, because of an incorrect offset.

Maybe zero sized cubes should be allowed, but that leads to the
discussion why cube([1,1,0]) is not a square, which leads to the
discussion why

     a=1;

     b=0;

     union(){

         cube([a,a,a]);

         cube([a,a,b]);

     }

results in a mixed 2D/3D warning. Put that in a more complex bit of
code, and you realize that the geometry warnings to not output
linenumbers, letalone a filename.


With kind regards,

Michael Frey


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

MichaelPFrey
In reply to this post by JordanBrown
On 05.02.19 02:42, Jordan Brown wrote:

You mean you can't say

    radius = (r != undef ) ? r : d/2;

?

defined with undef

    a = undef;

    echo(a);

ECHO: undef

is not the same as undefined:

    echo(b);

WARNING: Ignoring unknown variable 'b', in file ./, line 1.

ECHO: undef

The first case will not cause a warning.

The second case will. (but it would still function as normal)

I do not know which case you have.

----------------------

Do give a better example:

    module  test (d=1,r=undef){         radius = (r != undef ) ? r : d/2;         circle(radius);     }     test(r=5);     translate([10,0])     test(d=5);

that is fine.

-----------------

this is also fine:

module test (d=1,r=undef){ radius = !is_undef(r) ? r : d/2; circle(radius); } test(r=5); translate([10,0]) test(d=5);

-----------------

This now causes one warning:

module test (d=1){ radius = !is_undef(r) ? r : d/2; circle(radius); } test(r=5); translate([10,0]) test(d=5);

WARNING: variable r not specified as parameter, in file openscad, line 6

-----------------

This now causes two warnings:

    module  test (d=1){         radius = (r != undef ) ? r : d/2;         circle(radius);     }     test(r=5);     translate([10,0])     test(d=5);

WARNING: variable r not specified as parameter, in file openscad, line 6

WARNING: Ignoring unknown variable 'r', in file ./, line 2.

-----------------

note that all four examples create the same geometry.

This are not really good examples to show what is_undef() is mend for.


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

MichaelPFrey
In reply to this post by doug.moen
On 04.02.19 21:37, Doug Moen wrote:
Well, there is also this feature request: https://github.com/openscad/openscad/issues/1584

here is the work in progress:

https://github.com/openscad/openscad/pull/2762

https://github.com/openscad/openscad/pull/2761


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

JordanBrown
In reply to this post by MichaelPFrey
On 2/5/2019 10:35 AM, Michael Frey wrote:

Do give a better example:

    module  test (d=1,r=undef){
        radius = (r != undef ) ? r : d/2;
        circle(radius);
    }

    test(r=5);
    translate([10,0])
    test(d=5);

that is fine.


Good. I could work with that. My variant is a little different; I don't include the explicit defaulting to undef:

module test(r, d) {
	radius = (r != undef) ? r : d/2;
	circle(radius);
}

The intent is that you have to supply r or d. You could supply both, but then d would be ignored. (And I suppose if I was being obsessive, I'd have an assert to catch that.)  I can add an explicit "=undef" if I need to.

Not that anybody cares, but here's some of my source to produce a segment of a circle given some set of its parameters...

//             __E__
//          .-'  |  '-.
//        .'     |     '.
//       A-------D-------B
//      ;        |        ;
//      |        C        |
//      :                 ;
//       \               /
//        '.           .'
//          '-._____.-'
//
// r = CE, radius
// r1 = CD, distance from center to chord
// r2 = DE, distance from chord to circle
// c = AB, chord length
// arc = arc AEB, distance along arc
// angle = angle ACB
//
// I believe that given any two of those,
// you can get the rest.  However, I've
// only derived a few of the functions.
// 
function _radius(r, r1, r2, c, arc, angle) =
    r != undef ? r :
    r1 != undef && r2 != undef ? r1+r2 :
    r2 != undef && c != undef ? c*c/(8*r2) + r2/2 :
    undef;
function _radius2(r, r1, r2, c, arc, angle) =
    r2 != undef ? r2 :
    r != undef && r1 != undef ? r - r1 :
    undef;
function _radius1(r, r1, r2, c, arc, angle) =
    r1 != undef ? r1 :
    r != undef && r2 != undef ? r - r2 :
    r2 != undef && c != undef ? c*c/(8*r2) - r2/2 :
    undef;
function _angle(r, r1, r2, c, arc, angle) =
    angle != undef ? angle :
    r1 != undef && c != undef ? 2*atan2(c/2, r1) :
    r2 != undef && c != undef ? 2*atan2(c/2, c*c/(8*r2) - r2/2) :
    undef;

module segment(r, r1, r2, c, arc, angle) {
    _r = _radius(r=r, r1=r1, r2=r2, c=c, arc=arc, angle=angle);
    _r1 = _radius1(r=_r, r1=r1, r2=r2, c=c, arc=arc, angle=angle);
    intersection() {
        translate([0, -_r1]) circle(_r);
        translate([-_r, 0]) square(_r*2);
    }
}

I don't use the same variable names that the Wikipedia article uses, because I did the trig myself.  My trig was rusty so it was a lot of work, but it beat my previous scheme of setting a tape measure to a guestimated radius and swinging it to see if it matched the real-world curve.


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

MichaelPFrey
On 06.02.19 03:20, Jordan Brown wrote:

Good. I could work with that. My variant is a little different; I don't include the explicit defaulting to undef:

module test(r, d) {
	radius = (r != undef) ? r : d/2;
	circle(radius);
}

Just tested, that is also still fine.

If I remember correctly, the Ignoring Unknow Variable Warning for normal variables also exists/existed in OpenSCAD 2015.

New is, that special variables now also cause the "Ignoring Unknow Variable":

    https://github.com/openscad/openscad/issues/2468

    https://github.com/openscad/openscad/pull/2524

Some people relayed on that inconstancy and used undefined special variables.

That is where is_undef came from:

    https://github.com/openscad/openscad/pull/2535

Very specific workaround for a very specific issue, but is_undef can also be used for other purposes.

Special Variables still have a few inconsistency compared to normal variables.

Not that anybody cares, but here's some of my source to produce a segment of a circle given some set of its parameters...

Looks interesting and well done.

Given that I am lazy, I usually tend to use a circle and difference, which always gets messy, as I hand juggle variables.


wit kind regards,

Michael Frey


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

nophead
> Just tested, that is also still fine.

But why? The same code outside a module gives WARNING: Ignoring unknown variable 'r'. Why is it different inside a module?

On Wed, 6 Feb 2019 at 05:39, Michael Frey <[hidden email]> wrote:
On 06.02.19 03:20, Jordan Brown wrote:

Good. I could work with that. My variant is a little different; I don't include the explicit defaulting to undef:

module test(r, d) {
	radius = (r != undef) ? r : d/2;
	circle(radius);
}

Just tested, that is also still fine.

If I remember correctly, the Ignoring Unknow Variable Warning for normal variables also exists/existed in OpenSCAD 2015.

New is, that special variables now also cause the "Ignoring Unknow Variable":

    https://github.com/openscad/openscad/issues/2468

    https://github.com/openscad/openscad/pull/2524

Some people relayed on that inconstancy and used undefined special variables.

That is where is_undef came from:

    https://github.com/openscad/openscad/pull/2535

Very specific workaround for a very specific issue, but is_undef can also be used for other purposes.

Special Variables still have a few inconsistency compared to normal variables.

Not that anybody cares, but here's some of my source to produce a segment of a circle given some set of its parameters...

Looks interesting and well done.

Given that I am lazy, I usually tend to use a circle and difference, which always gets messy, as I hand juggle variables.


wit kind regards,

Michael Frey

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

JordanBrown
On 2/6/2019 1:32 AM, nop head wrote:
> Just tested, that is also still fine.

But why? The same code outside a module gives WARNING: Ignoring unknown variable 'r'. Why is it different inside a module?


My guess:  Inside a module (or function) you have the variable "declared" in the argument list.  You can be pretty sure that it's not a typo.  Outside, it's a variable that you've never seen before... is it a typo?


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

nophead
Yes I suppose the parameter as been named so it exists and it value is undef as opposed to not existing at all. It is all a bit confusing though, even for an experienced programmer. The subtle difference between undefined and defined but not given a value, or assigned and given the value undef. I have replaced == undef with is_undef() in more places than I needed to.

Before these undef warning were added the language was simpler and easier to understand, but harder to debug.

On Wed, 6 Feb 2019 at 17:59, Jordan Brown <[hidden email]> wrote:
On 2/6/2019 1:32 AM, nop head wrote:
> Just tested, that is also still fine.

But why? The same code outside a module gives WARNING: Ignoring unknown variable 'r'. Why is it different inside a module?


My guess:  Inside a module (or function) you have the variable "declared" in the argument list.  You can be pretty sure that it's not a typo.  Outside, it's a variable that you've never seen before... is it a typo?


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

Ronaldo
undef still is a valid value. A variable is known if it was initialized or it is a function or module argument in the scope it is used. OpenSCAD assigns an undef value to a non-initialized variable (so unknown) and a warning for this is welcome IMO to easy the catch of a typo. What may cause confusion is to call undefined a variable that was not initialized. Variables initialized with the value undef should not generate a warning. It is not clear for me if is_undef() checks its argument value or if its  argument variable was initialized. I think those are different situations and worth of distinct test functions.

Em qua, 6 de fev de 2019 às 18:47, nop head <[hidden email]> escreveu:
Yes I suppose the parameter as been named so it exists and it value is undef as opposed to not existing at all. It is all a bit confusing though, even for an experienced programmer. The subtle difference between undefined and defined but not given a value, or assigned and given the value undef. I have replaced == undef with is_undef() in more places than I needed to.

Before these undef warning were added the language was simpler and easier to understand, but harder to debug.

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

nophead
From testing is_undef is true for both cases. I don't think we need to differentiate between them do we? It is just odd to me one gives a warning and the other doesn't. I.e. I thought I would always need to use is_undef and that == undef would always give a warning. 

On Wed, 6 Feb 2019 at 20:28, Ronaldo Persiano <[hidden email]> wrote:
undef still is a valid value. A variable is known if it was initialized or it is a function or module argument in the scope it is used. OpenSCAD assigns an undef value to a non-initialized variable (so unknown) and a warning for this is welcome IMO to easy the catch of a typo. What may cause confusion is to call undefined a variable that was not initialized. Variables initialized with the value undef should not generate a warning. It is not clear for me if is_undef() checks its argument value or if its  argument variable was initialized. I think those are different situations and worth of distinct test functions.

Em qua, 6 de fev de 2019 às 18:47, nop head <[hidden email]> escreveu:
Yes I suppose the parameter as been named so it exists and it value is undef as opposed to not existing at all. It is all a bit confusing though, even for an experienced programmer. The subtle difference between undefined and defined but not given a value, or assigned and given the value undef. I have replaced == undef with is_undef() in more places than I needed to.

Before these undef warning were added the language was simpler and easier to understand, but harder to debug.
_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

Ronaldo
From testing is_undef is true for both cases. I don't think we need to differentiate between them do we? It is just odd to me one gives a warning and the other doesn't. I.e. I thought I would always need to use is_undef and that == undef would always give a warning. 

As far I understand , is_undef() was created to avoid warnings with non-initialized special variables. It would be better to define something like is_init(x) which returns false iff the variable x was not initialized without issuing any warning. And leave undef in peace. 

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

MichaelPFrey
On 07.02.19 00:16, Ronaldo Persiano wrote:
As far I understand , is_undef() was created to avoid warnings with non-initialized special variables.

That is correct.

But being consequent, is_undef() also avoids warnings with non-initialized normal variables.


To be clear: The Ignoring unknown variable is almost 10 years old:

https://github.com/openscad/openscad/blob/ed687599bf1ca36869dc64dab2b9969f1d96b4c1/context.cc#L61-L75

... which makes me wonder why the warning is written as "Ignoring unkown variable".

It does not really ignore unknown, it just defaults to undef.


The same message for special variables is recent:

https://github.com/openscad/openscad/pull/2524


It would be better to define something like is_init(x) which returns false if the variable x was not initialized without issuing any warning. And leave undef in peace.

Interesting idea.

The reason why I picked is_undef was, that it is straightforward to implement:

https://github.com/openscad/openscad/pull/2535/files

lookup_variable already has/had a silent flag for many many year.

To implement is_init, one would have to duplicate lookup_variable.

Not as straight forward, but doable. I will think about the implications.


With kind regards,

Michael Frey


_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org
Reply | Threaded
Open this post in threaded view
|

Re: A warning too far?

nophead
How can you create a variable that isn't initialised? Or do you mean a function parameter?

Then we would have two functions replacing the simple == undef and would have to negate them for != undef. I don't think this is progress. == and != undef should have just been made silent. That has worked fine in the past as is simple to understand.

With the latest master I found two places where I replaced len() with is_list() but three places where I needed is_list() && len() so I created Len() to do that. So my code is longer and slower to do the same thing and OpenSCAD is bigger. Also the documentation for len() says that it returns undef for scalars. It was well defined, so I don't think it should give a warning when passed a scalar.









On Thu, 7 Feb 2019 at 05:53, Michael Frey <[hidden email]> wrote:
On 07.02.19 00:16, Ronaldo Persiano wrote:
As far I understand , is_undef() was created to avoid warnings with non-initialized special variables.

That is correct.

But being consequent, is_undef() also avoids warnings with non-initialized normal variables.


To be clear: The Ignoring unknown variable is almost 10 years old:

https://github.com/openscad/openscad/blob/ed687599bf1ca36869dc64dab2b9969f1d96b4c1/context.cc#L61-L75

... which makes me wonder why the warning is written as "Ignoring unkown variable".

It does not really ignore unknown, it just defaults to undef.


The same message for special variables is recent:

https://github.com/openscad/openscad/pull/2524


It would be better to define something like is_init(x) which returns false if the variable x was not initialized without issuing any warning. And leave undef in peace.

Interesting idea.

The reason why I picked is_undef was, that it is straightforward to implement:

https://github.com/openscad/openscad/pull/2535/files

lookup_variable already has/had a silent flag for many many year.

To implement is_init, one would have to duplicate lookup_variable.

Not as straight forward, but doable. I will think about the implications.


With kind regards,

Michael Frey

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org

_______________________________________________
OpenSCAD mailing list
[hidden email]
http://lists.openscad.org/mailman/listinfo/discuss_lists.openscad.org