Using choices with valued-boolean variable

[victorsndvg]

Hi @szaghi ,

we are trying to use the choices argument of the cli%add subroutine with a non required boolean value. We expected to define the allowed "boolean strings" when using this argument, but we are not getting the expected behaviour.

You can see a piece of code with our test case in the following link:
https://gist.github.com/victorsndvg/91d90aee7a3cf5917734

If the optional CLA --bolean-value is not present we get a error message when calling cli%get:
test_FLAP_boolean_choices: error: value of named option "--boolean-value" must be chosen in: (.True.,.False.) "" has been passed!

We also get the same error message if we use any valid "boolean string" (.True., .False., true, T, etc.) even when using one of the choices.

Finally, if you use a string that cannot be converted to a boolean variable we get this message:
test_FLAP_boolean_choices: error: cannot convert "asdf" of option "--boolean-value" to logical type!

What do you think?
What is the expected behaviour?

[szaghi]

@victorsndvg it seem a bug. I have few minutes now, I am doing a rapid check. Thanks for reporting.

[szaghi]

@victorsndvg It seem that I have missed the choices check for boolean type... I try to fix this night...

[szaghi]

@victorsndvg sorry... but I just realized why I do not implement the choices check for logical... the logical flag are limited by default in true or false... the meaning of choices attributes is to limit the flag value in smaller range with respect the whole possibilities, a shorter range of (true or false) is... true or false.

What do you want achieve limiting a boolean flag?

[szaghi]

@victorsndvg to be more clear. In my mind it has sense a choices for numbers or string like

• set i for i in (3, 6, 9);
• set s for s in ("home ", "house ", "buildings")

On the contrary, why limit a variable that is already limited in between (.true., .false.)?

Maybe you were thinking to an array of boolean?

[victorsndvg]

Thank you for the quick response @szaghi ,

Ok you are right, I was seeing the code and now I know how the booleans are beeing converted from strings.

Maybe is a nonsense to limit the "boolean-string-values", if this is true, I think that the expected behaviour could be to check that choices was not present when adding argument at the moment of getting (cli%get) boolean values. Do you agree?

Other secondary problem is that user doesn't know what strings are a "valid-booleans". What is the best way to show this to the user with FLAP?

[szaghi]

@victorsndvg Good points!

• adding the check that choices should not be specified for logic value is straightforward (in the get method as you said);
• the valid boolean string values are all valid fortran logical values that can be read, namely .true. or t and equivalent for false; I do not know how to clearly emphasize this in the runtime help: when you select action="store" you can store anything not only logical, thus customize the help message is difficult; maybe I can improve the documenration.

Any suggestions is welcome. Thank you victor.

[victorsndvg]

Hi @szaghi,

adding the check that choices should not be specified for logic value is straightforward (in the get method as you said);

In my opinion, this would be of paramount importance. In the current status of FLAP, if you specify a set of choices= in the %add() TBP for a boolean command-line-parameter (e.g., choices=".true.,.false."), then the further %get() call does not work anymore (independently of whether you are explicitly providing this parameter or not to the program).

Thanks!

[szaghi]

@victorsndvg ok, let me few hours and it will come very soon.

[szaghi]

@victorsndvg

Done. Now there is a test for this.

If you run the test you will obtain:

test_choices_logical: error: cannot use "choices" value check for option "--boolean-value" it being of logical type! The choices is, by definition of logical, limited to ".true." or ".false."

Error code: 11

The error message should be clearly indicate to the user that using choices within logical options is not allowed.

Let me know if this is ok for you.

See you soon.

P.S. Note that I have modified other projects aspects: library and tests are now separated, and the fobos has been modified in order to compile all test at once.

[victorsndvg]

Thank you @szaghi , good work!

I'm going to update my FLAP fork ;)