Added typed defaults, print_default/print_choices and tests#17
Added typed defaults, print_default/print_choices and tests#17lukaswittmann wants to merge 7 commits intochselz:mainfrom
Conversation
…defaults+choices), and wp real handling (for now only dp) Signed-off-by: lukaswittmann <mail@lukaswittmann.com>
There was a problem hiding this comment.
Pull request overview
This pull request enhances the fclap command-line argument parser library with typed default values, improved help formatting, and comprehensive test coverage. The changes enable arguments to accept typed defaults (integer, real, logical, string) via polymorphic class(*) parameters, introduce print_default and print_choices toggles for help text control, and switch real-value handling to working precision (real(wp)) to avoid precision loss. The PR also adds dual real getter support for both real and real(wp) outputs, implements strict type validation for mismatched defaults, and provides refined real number display formatting.
Changes:
- Enhanced
add_argumentto accept typed defaults via polymorphicdefault_valparameter with runtime type validation - Added
print_defaultandprint_choicesboolean flags to control help text generation - Migrated all real storage and parsing to
real(wp)(working precision) with dual getter overloads for backward compatibility - Implemented sophisticated real number formatting (exponential for |value| < 1.0, fixed for >= 1.0, with trailing zero trimming)
- Added 13 new unit tests covering typed defaults, help formatting, precision handling, and getter overloads
- Added 17 deferred failure tests (disabled by default) for error path coverage once error handling is improved
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/main.f90 | Added comprehensive tests for typed defaults, help formatting toggles, real precision, getter overloads, and deferred failure scenarios |
| src/fclap/fclap_parser.f90 | Changed default_val from string to class(*), added type validation, integrated print_default/print_choices parameters |
| src/fclap/fclap_namespace.f90 | Converted real storage to real(wp), added format_real_for_display and trim_real_fraction utilities, implemented dual real getters |
| src/fclap/fclap_formatter.f90 | Added format_action_help and format_action_choices helpers to generate help text with defaults and choices |
| src/fclap/fclap_actions.f90 | Added print_default/print_choices fields, improved error messages with with_argument_context helper, converted to real(wp) |
| src/fclap.f90 | Exported wp (working precision kind) for public use |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: lukaswittmann <mail@lukaswittmann.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lukas Wittmann <wittmann@uni-bonn.de>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lukas Wittmann <wittmann@uni-bonn.de>
Signed-off-by: lukaswittmann <mail@lukaswittmann.com>
| procedure, private :: get_sub_integer => namespace_get_sub_integer | ||
| procedure, private :: get_sub_real => namespace_get_sub_real | ||
| procedure, private :: get_sub_real_sp => namespace_get_sub_real_sp | ||
| procedure, private :: get_sub_real_wp => namespace_get_sub_real_wp |
There was a problem hiding this comment.
i dont get why there is a special function for sp as wp stands for working /wanted precision and not as double precision. in accuracy.f90 it is defined as wp=dp but there one can also change it to single precision
There was a problem hiding this comment.
For the case that a user inputs a sp default and our wp is dp (or differs)
Edit: You are right: With both get_sub_real_sp and get_sub_real_wp in the same generic, one could hit a bad case when wp equals default real kind (I dont know what would happen in such a case), but the two specifics become effectively the same interface.
I am fixing it.
Edit2: With the new API, a call like args%get(..., sp_var) will fail at compile time when wp=dp. I am unsure if this is the best solution using wp (as the core API) and having users set it to the desired kind. I have created #18 to discuss this.
chselz
left a comment
There was a problem hiding this comment.
in principle LGTM but please check comments regarding sp vs wp (per default dp but not necessarily)
Signed-off-by: lukaswittmann <mail@lukaswittmann.com>
Signed-off-by: lukaswittmann <mail@lukaswittmann.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
General Changes
add_argumentdefaults to accept typed values viaclass(*)(integer, real, logical, string)print_default(default.true.): prints(default: ...)in helppage only when a default is setprint_choices(default.false.): optionally print choices as[choices: ...]'value')real(wp)) across parsing/storage/getters to avoid precision loss in defaults/help outputargs%getfor bothrealandreal(wp)outputsdefault_valtypes are now rejected atadd_argumenttime and stored in parser error state< 1.0-> exponential,>= 1.0-> fixed,.0minimum)Tests
print_default/print_choicestoggles, quoted string choices, help alignment, real formatting/precision (wp), dual real getter overloads, and strict mismatched-default rejection checksImportant
RUN_DEFERRED_FAILURE_TESTS = .false.) for error-path scenarios to enable once parse errors are returned instead of exiting: missing required args, missing values, wrong types, invalid choices, mutex violations, bound violations, unknown option/subcommand, extra positional args, removed args, and append-without-valueSee issue #16 and the needed previous rework in #8 and especially #12