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.

Sign up for GitHub

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

Jump to bottom

[MISC] Change value_list_validator construction. #1298

Merged
merged 1 commit into from
Oct 29, 2019
Merged

[MISC] Change value_list_validator construction. #1298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ If possible, provide tooling that performs the changes, e.g. a shell-script.

#### Argument parser
* Simplified reading file extensions from formatted files in the input/output file validators.
* The seqan3::value_list_validator is now constructible from a range or a parameter pack.
* Enable subcommand argument parsing ([How-to](https://docs.seqan.de/seqan/3-master-user/subcommand_arg_parse.html)).

#### Core
Expand All @@ -42,6 +43,8 @@ If possible, provide tooling that performs the changes, e.g. a shell-script.

#### Argument parser

* The seqan3::value_list_validator is not constructible from a std::initialiser_list any more
(e.g. `seqan3::value_list_validator{{1,2,3}}` does **not** work, use `seqan3::value_list_validator{1,2,3}` instead).
* **Changed class signature of input/output file validators:**
Most user code will be unaffected; to fix possible compiler errors you need to add an empty template parameter list to
the respective instances (e.g. change `input_file_validator` to `input_file_validator<>`).
Expand Down
2 changes: 1 addition & 1 deletion doc/tutorial/argument_parser/solution6.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ void initialize_argument_parser(argument_parser & parser, cmd_arguments & args)

//![value_list_validator]
parser.add_option(args.aggregate_by, 'a', "aggregate-by", "Choose your method of aggregation.",
option_spec::DEFAULT, value_list_validator{{"median", "mean"}});
option_spec::DEFAULT, value_list_validator{"median", "mean"});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general these files seem not to follow our style guide which they actually should but that is for another time. It seems we need to go through the tutorials anyway before the release. But maybe you can mark it already as a card so we do not forget about it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which style do you mean specifically?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the all-or-nothing rule:
if you break on a line, then put every argument on a separate line

parser.add_option(args.aggregate_by, 
                  'a', 
                  "aggregate-by", 
                  "Choose your method of aggregation.",
                  option_spec::DEFAULT, 
                  value_list_validator{{"median", "mean"}});

//![value_list_validator]

parser.add_flag(args.header_is_set, 'H', "header-is-set", "Let us know whether your data file contains a "
Expand Down
100 changes: 79 additions & 21 deletions include/seqan3/argument_parser/validators.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <seqan3/argument_parser/exceptions.hpp>
#include <seqan3/core/concept/core_language.hpp>
#include <seqan3/core/detail/to_string.hpp>
#include <seqan3/core/type_list/traits.hpp>
#include <seqan3/core/type_traits/basic.hpp>
#include <seqan3/core/type_traits/pre.hpp>
#include <seqan3/io/detail/misc.hpp>
Expand Down Expand Up @@ -138,7 +139,7 @@ class arithmetic_range_validator
*/
template <std::ranges::forward_range range_type>
//!\cond
requires arithmetic<value_type_t<range_type>>
requires arithmetic<std::ranges::range_value_t<range_type>>
//!\endcond
void operator()(range_type const & range) const
{
Expand All @@ -165,10 +166,14 @@ class arithmetic_range_validator
*
* \details
*
* On construction, the validator must receive a list (vector) of valid values.
* On construction, the validator must receive a range or parameter pack of valid values.
* The struct than acts as a functor, that throws a seqan3::parser_invalid_argument
* exception whenever a given value is not in the given list.
*
* \note In order to simplify the chaining of validators, the option value type is deduced to `double` for ranges whose
* value type models seqan3::arithmetic, and to `std::string` if the ranges value type is convertible to it.
* Otherwise, the option value type is deduced to the value type of the range.
*
* \include test/snippet/argument_parser/validators_2.cpp
*/
template <typename option_value_type>
Expand All @@ -178,11 +183,45 @@ class value_list_validator
//!\brief Type of values that are tested by validator
using value_type = option_value_type;

/*!\brief Constructing from a vector.
* \param[in] v The vector of valid values to test.
/*!\name Constructors, destructor and assignment
* \{
*/
value_list_validator(std::vector<value_type> v) : values{std::move(v)}
{}
value_list_validator() = default; //!< Defaulted.
value_list_validator(value_list_validator const &) = default; //!< Defaulted.
value_list_validator(value_list_validator &&) = default; //!< Defaulted.
value_list_validator & operator=(value_list_validator const &) = default; //!< Defaulted.
value_list_validator & operator=(value_list_validator &&) = default; //!< Defaulted.
~value_list_validator() = default; //!< Defaulted.

/*!\brief Constructing from a range.
* \tparam range_type The type of range; must model std::ranges::forward_range and value_list_validator::value_type
* must be constructible from the rvalue reference type of the given range.
* \param[in] rng The range of valid values to test.
*/
template <std::ranges::forward_range range_type>
//!\cond
requires std::constructible_from<option_value_type, std::ranges::range_rvalue_reference_t<range_type>>
//!\endcond
value_list_validator(range_type rng)
{
values.clear();
std::ranges::move(std::move(rng), std::ranges::back_inserter(values));
}

/*!\brief Constructing from a parameter pack.
* \tparam option_types The type of option values in the parameter pack; The value_list_validator::value_type must
* be constructible from each type in the parameter pack.
* \param[in] opts The parameter pack values.
*/
template <typename ...option_types>
//!\cond
requires (std::constructible_from<option_value_type, option_types> && ...)
//!\endcond
value_list_validator(option_types && ...opts)
{
(values.emplace_back(std::forward<option_types>(opts)), ...);
}
//!\}

/*!\brief Tests whether cmp lies inside values.
* \param cmp The input value to check.
Expand All @@ -201,11 +240,11 @@ class value_list_validator
*/
template <std::ranges::forward_range range_type>
//!\cond
requires std::convertible_to<value_type_t<range_type>, value_type const &>
requires std::convertible_to<std::ranges::range_value_t<range_type>, option_value_type>
//!\endcond
void operator()(range_type const & range) const
{
std::for_each(range.begin(), range.end(), [&] (auto cmp) { (*this)(cmp); });
std::for_each(std::ranges::begin(range), std::ranges::end(range), [&] (auto cmp) { (*this)(cmp); });
}

//!\brief Returns a message that can be appended to the (positional) options help page info.
Expand All @@ -217,26 +256,45 @@ class value_list_validator
private:

//!\brief Minimum of the range to test.
std::vector<value_type> values;
std::vector<value_type> values{};
};

/*!\brief Type deduction guides
* \relates seqan3::value_list_validator
* \{
*/
//!\brief Deduction guide for `std::vector` over an arithmetic type.
template <arithmetic option_value_type>
value_list_validator(std::vector<option_value_type>) -> value_list_validator<double>;
//!\brief Deduction guide for a parameter pack over an arithmetic type.
template <arithmetic ...option_types>
value_list_validator(option_types...) -> value_list_validator<double>;

//!\brief Deduction guide for `std::initializer_list` over an arithmetic type.
template <arithmetic option_value_type>
value_list_validator(std::initializer_list<option_value_type>) -> value_list_validator<double>;
//!\brief Deduction guide for ranges over an arithmetic type.
template <std::ranges::forward_range range_type>
//!\cond
requires arithmetic<std::ranges::range_value_t<range_type>>
//!\endcond
value_list_validator(range_type && rng) -> value_list_validator<double>;

//!\brief Given a parameter pack of types that are convertible to std::string, delegate to value type std::string.
template <typename ...option_types>
//!\cond
requires (std::constructible_from<std::string, option_types> && ...)
//!\endcond
value_list_validator(option_types...) -> value_list_validator<std::string>;

//!\brief Deduction guide for ranges over a value type convertible to std::string.
template <std::ranges::forward_range range_type>
//!\cond
requires std::constructible_from<std::string, std::ranges::range_value_t<range_type>>
//!\endcond
value_list_validator(range_type && rng) -> value_list_validator<std::string>;

//!\brief Deduction guide for `std::vector` over `const char *`.
value_list_validator(std::vector<const char *>) -> value_list_validator<std::string>;
//!\brief Deduction guide for a parameter pack.
template <typename ...option_types>
value_list_validator(option_types...) -> value_list_validator<seqan3::pack_traits::front<option_types...>>;

//!\brief Deduction guide for `std::initializer_list` over `const char *`.
value_list_validator(std::initializer_list<const char *>) -> value_list_validator<std::string>;
//!\brief Deduction guide for ranges.
template <std::ranges::forward_range range_type>
value_list_validator(range_type && rng) -> value_list_validator<std::ranges::range_value_t<range_type>>;
//!\}

/*!\brief An abstract base class for the file and directory validators.
Expand Down Expand Up @@ -287,7 +345,7 @@ class file_validator_base
*/
template <std::ranges::forward_range range_type>
//!\cond
requires std::convertible_to<value_type_t<range_type>, std::filesystem::path const &>
requires std::convertible_to<std::ranges::range_value_t<range_type>, std::filesystem::path const &>
//!\endcond
void operator()(range_type const & v) const
{
Expand Down Expand Up @@ -808,7 +866,7 @@ class regex_validator
*/
template <std::ranges::forward_range range_type>
//!\cond
requires std::convertible_to<value_type_t<range_type>, value_type const &>
requires std::convertible_to<std::ranges::range_value_t<range_type>, value_type const &>
//!\endcond
void operator()(range_type const & v) const
{
Expand Down
2 changes: 1 addition & 1 deletion test/snippet/argument_parser/validators_2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ int main(int argc, const char ** argv)

//![validator_call]
int myint;
seqan3::value_list_validator my_validator{{2, 4, 6, 8, 10}};
seqan3::value_list_validator my_validator{2, 4, 6, 8, 10};

myparser.add_option(myint,'i',"integer","Give me a number.",
seqan3::option_spec::DEFAULT, my_validator);
Expand Down
50 changes: 38 additions & 12 deletions test/unit/argument_parser/format_parse_validators_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <seqan3/alphabet/all.hpp>
#include <seqan3/core/char_operations/predicate.hpp>
#include <seqan3/range/views/persist.hpp>
#include <seqan3/range/views/take.hpp>
#include <seqan3/std/filesystem>
#include <seqan3/test/tmp_filename.hpp>

Expand Down Expand Up @@ -689,18 +690,44 @@ TEST(validator_test, arithmetic_range_validator_error)
EXPECT_THROW(parser7.parse(), validation_failed);
}

enum class foo
{
one,
two,
three
};

TEST(validator_test, value_list_validator_success)
{
// type deduction
// --------------
// all arithmetic types are deduced to double in order to easily allow chaining of arithmetic validators
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This explanation could also go into the class docs. I was actually wondering why it's double for all arithmetic types...

EXPECT_TRUE((std::same_as<value_list_validator<double>, decltype(value_list_validator{1})>));
// The same holds for a range of arithmetic types
std::vector v{1, 2, 3};
EXPECT_TRUE((std::same_as<value_list_validator<double>, decltype(value_list_validator{v})>));
EXPECT_TRUE((std::same_as<value_list_validator<double>, decltype(value_list_validator{v | views::take(2)})>));
// const char * is deduced to std::string
std::vector v2{"ha", "ba", "ma"};
EXPECT_TRUE((std::same_as<value_list_validator<std::string>, decltype(value_list_validator{"ha", "ba", "ma"})>));
EXPECT_TRUE((std::same_as<value_list_validator<std::string>, decltype(value_list_validator{v2})>));
EXPECT_TRUE((std::same_as<value_list_validator<std::string>, decltype(value_list_validator{v2 | views::take(2)})>));
// custom types are used as is
EXPECT_TRUE((std::same_as<value_list_validator<foo>, decltype(value_list_validator{foo::one, foo::two})>));

// usage
// -----
std::string option_value;
int option_value_int;
std::vector<std::string> option_vector;
std::vector<int> option_vector_int;

// option
std::vector<std::string> valid_str_values{"ha", "ba", "ma"};
const char * argv[] = {"./argument_parser_test", "-s", "ba"};
argument_parser parser{"test_parser", 3, argv, false};
parser.add_option(option_value, 's', "string-option", "desc",
option_spec::DEFAULT, value_list_validator{{"ha", "ba", "ma"}});
option_spec::DEFAULT, value_list_validator{valid_str_values | views::take(2)});

testing::internal::CaptureStderr();
EXPECT_NO_THROW(parser.parse());
Expand All @@ -711,7 +738,7 @@ TEST(validator_test, value_list_validator_success)
const char * argv2[] = {"./argument_parser_test", "-i", "-21"};
argument_parser parser2{"test_parser", 3, argv2, false};
parser2.add_option(option_value_int, 'i', "int-option", "desc",
option_spec::DEFAULT, value_list_validator<int>{{0, -21, 10}});
option_spec::DEFAULT, value_list_validator<int>{0, -21, 10});

testing::internal::CaptureStderr();
EXPECT_NO_THROW(parser2.parse());
Expand All @@ -721,7 +748,7 @@ TEST(validator_test, value_list_validator_success)
// positional option
const char * argv3[] = {"./argument_parser_test", "ma"};
argument_parser parser3{"test_parser", 2, argv3, false};
parser3.add_positional_option(option_value, "desc", value_list_validator{{"ha", "ba", "ma"}});
parser3.add_positional_option(option_value, "desc", value_list_validator{valid_str_values});

testing::internal::CaptureStderr();
EXPECT_NO_THROW(parser3.parse());
Expand All @@ -731,8 +758,7 @@ TEST(validator_test, value_list_validator_success)
// positional option - vector
const char * argv4[] = {"./argument_parser_test", "ha", "ma"};
argument_parser parser4{"test_parser", 3, argv4, false};
parser4.add_positional_option(option_vector, "desc",
value_list_validator{{"ha", "ba", "ma"}});
parser4.add_positional_option(option_vector, "desc", value_list_validator{"ha", "ba", "ma"});

testing::internal::CaptureStderr();
EXPECT_NO_THROW(parser4.parse());
Expand All @@ -744,7 +770,7 @@ TEST(validator_test, value_list_validator_success)
const char * argv5[] = {"./argument_parser_test", "-i", "-10", "-i", "48"};
argument_parser parser5{"test_parser", 5, argv5, false};
parser5.add_option(option_vector_int, 'i', "int-option", "desc",
option_spec::DEFAULT, value_list_validator<int>{{-10,48,50}});
option_spec::DEFAULT, value_list_validator<int>{-10, 48, 50});

testing::internal::CaptureStderr();
EXPECT_NO_THROW(parser5.parse());
Expand All @@ -757,7 +783,7 @@ TEST(validator_test, value_list_validator_success)
const char * argv7[] = {"./argument_parser_test", "-h"};
argument_parser parser7{"test_parser", 2, argv7, false};
parser7.add_option(option_vector_int, 'i', "int-option", "desc",
option_spec::DEFAULT, value_list_validator<int>{{-10,48,50}});
option_spec::DEFAULT, value_list_validator<int>{-10, 48, 50});

option_vector_int.clear();
testing::internal::CaptureStdout();
Expand All @@ -767,7 +793,7 @@ TEST(validator_test, value_list_validator_success)
"===========" +
basic_options_str +
" -i, --int-option (List of signed 32 bit integer's)"
" desc Default: []. Value must be one of [-10,48,50]." +
" desc Default: []. Value must be one of [-10, 48, 50]." +
basic_version_str);
EXPECT_TRUE(ranges::equal((my_stdout | std::views::filter(!is_space)),
expected | std::views::filter(!is_space)));
Expand All @@ -784,30 +810,30 @@ TEST(validator_test, value_list_validator_error)
const char * argv[] = {"./argument_parser_test", "-s", "sa"};
argument_parser parser{"test_parser", 3, argv, false};
parser.add_option(option_value, 's', "string-option", "desc",
option_spec::DEFAULT, value_list_validator{{"ha", "ba", "ma"}});
option_spec::DEFAULT, value_list_validator{"ha", "ba", "ma"});

EXPECT_THROW(parser.parse(), validation_failed);

// positional option
const char * argv3[] = {"./argument_parser_test", "30"};
argument_parser parser3{"test_parser", 2, argv3, false};
parser3.add_positional_option(option_value_int, "desc", value_list_validator{{0, 5, 10}});
parser3.add_positional_option(option_value_int, "desc", value_list_validator{0, 5, 10});

EXPECT_THROW(parser3.parse(), validation_failed);

// positional option - vector
const char * argv4[] = {"./argument_parser_test", "fo", "ma"};
argument_parser parser4{"test_parser", 3, argv4, false};
parser4.add_positional_option(option_vector, "desc",
value_list_validator{{"ha", "ba", "ma"}});
value_list_validator{"ha", "ba", "ma"});

EXPECT_THROW(parser4.parse(), validation_failed);

// option - vector
const char * argv5[] = {"./argument_parser_test", "-i", "-10", "-i", "488"};
argument_parser parser5{"test_parser", 5, argv5, false};
parser5.add_option(option_vector_int, 'i', "int-option", "desc",
option_spec::DEFAULT, value_list_validator<int>{{-10,48,50}});
option_spec::DEFAULT, value_list_validator<int>{-10, 48, 50});

EXPECT_THROW(parser5.parse(), validation_failed);
}
Expand Down