Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Using choices with valued-boolean variable #51

Closed
victorsndvg opened this issue Sep 9, 2015 · 10 comments
Closed

Using choices with valued-boolean variable #51

victorsndvg opened this issue Sep 9, 2015 · 10 comments
Labels

Comments

@victorsndvg
Copy link

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
Copy link
Owner

szaghi commented Sep 9, 2015

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

@szaghi szaghi added the bug label Sep 9, 2015
@szaghi
Copy link
Owner

szaghi commented Sep 9, 2015

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

@szaghi
Copy link
Owner

szaghi commented Sep 9, 2015

@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
Copy link
Owner

szaghi commented Sep 9, 2015

@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
Copy link
Author

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
Copy link
Owner

szaghi commented Sep 9, 2015

@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
Copy link
Author

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
Copy link
Owner

szaghi commented Sep 10, 2015

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

@szaghi
Copy link
Owner

szaghi commented Sep 11, 2015

@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.

@szaghi szaghi closed this as completed Sep 11, 2015
@victorsndvg
Copy link
Author

Thank you @szaghi , good work!

I'm going to update my FLAP fork ;)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants