[MISC] Change seqan3::option_spec enum names to lower case. #2285

smehringer · 2020-11-26T15:39:55Z

I had some free time to tackle other stuff and this was something API relevant that needed to be done for a while.

The enum names of seqan3::option_spec were changed to lower case:

seqan3::option_spec::DEFAULT is replaced by seqan3::option_spec::defaulted.
seqan3::option_spec::REQUIRED is replaced by seqan3::option_spec::required.
seqan3::option_spec::ADVANCED is replaced by seqan3::option_spec::advanced.
seqan3::option_spec::HIDDEN is replaced by seqan3::option_spec::hidden.

codecov · 2020-11-26T16:17:00Z

Codecov Report

Merging #2285 (3226f4d) into master (d797403) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2285   +/-   ##
=======================================
  Coverage   98.18%   98.19%           
=======================================
  Files         261      262    +1     
  Lines       10820    10853   +33     
=======================================
+ Hits        10624    10657   +33     
  Misses        196      196

Impacted Files	Coverage Δ
include/seqan3/argument_parser/auxiliary.hpp	`100.00% <ø> (ø)`
include/seqan3/argument_parser/exceptions.hpp	`100.00% <ø> (ø)`
include/seqan3/argument_parser/argument_parser.hpp	`98.77% <100.00%> (-0.01%)`	⬇️
...lude/seqan3/argument_parser/detail/format_base.hpp	`89.47% <100.00%> (ø)`
...ude/seqan3/argument_parser/detail/format_parse.hpp	`96.80% <100.00%> (ø)`
include/seqan3/core/simd/view_to_simd.hpp	`91.80% <0.00%> (ø)`
include/seqan3/core/detail/debug_stream_range.hpp	`100.00% <0.00%> (ø)`
...nclude/seqan3/core/configuration/configuration.hpp	`100.00% <0.00%> (ø)`
...clude/seqan3/core/detail/debug_stream_alphabet.hpp	`100.00% <0.00%> (ø)`
...qan3/alignment/pairwise/alignment_configurator.hpp	`97.67% <0.00%> (ø)`
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d64e36...3226f4d. Read the comment docs.

simonsasse

Looks Good, just wondered if seqan3::option_spec::default might be more straight forward ? 🥳

include/seqan3/argument_parser/auxiliary.hpp

smehringer · 2020-12-02T13:59:51Z

Looks Good, just wondered if seqan3::option_spec::default might be more straight forward ? partying_face

default is a reserved name because it is a compiler keyword so I cannot use it.
E.g. in constructors we write foo() = default;

rrahn

Thank you. The defaulted -> default naming should be addressed. Maybe you can independently of me research about the meaning defaulted as an adjective. If I am not mistaken, then we have this all over the library 😢

CHANGELOG.md

doc/tutorial/argument_parser/index.md

rrahn · 2020-12-05T13:32:23Z

doc/tutorial/argument_parser/index.md

-| REQUIRED   | Required options will cause an error if not provided.          |
-| ADVANCED   | Advanced options are only displayed wit `-hh/--advanced-help`. |
-| HIDDEN     | Hidden options are never displayed when exported.              |
+| defaulted  | The default tag with no special behaviour.                     |


I think the meaning of defaulted as adjective is wrong here. So default was actually correct.
And I just realise that we use this misleading word for all the defaulted constructors 🤯.

Again, I cannot use default :( ! (in lower case letters) because it is a compiler keyword. So I thought defaulted is the next best alternative. If you deem defaulted wrong, how else should we name it?

default as we use it, is already an adjective. But you can also see it as a verb to default and then defaulted is the past tense that can still mean something was set to default. (see https://www.dict.cc/englisch-deutsch/to+default.html, translating to default to etw. als Standardeinstellung übernehmen)

But the meaning is rather: required option, hidden option etc. As far as I remember, we use a verb for functions only (it would be good if I could point now to the style guide but we don't have it). Nonetheless, I never've seen an enum value as a verb so it would be confusing to start now. Another, question, why do we have a default tag? Would that not be something internal, if the user just does not specify anything? What is the default behaviour? Why do you need to say explicitly it is default, which is already the meaning if I don't specify the other? Maybe we can remove it?

Nonetheless, I never've seen an enum value as a verb so it would be confusing to start now.

hm.. I see. I have no strong feelings about this.

Another, question, why do we have a default tag?

Because you need to specify the option_spec if you want to specify a validator.

p.add_option(var, 'i', "integer", "desc", option_spec::default, arithmetic_validator{3, 5});

Maybe we can remove it?

Only if we would add an add_option overload that omits the option_spec before the validator. 🤷
I don't know if we want to add several overloads to allow "a different order".

What is the default behaviour?

Neither, required, advanced nor hidden. Just a normal option.

Possible alternatives (meaning "nothing special"): standard, normal, simple, orderly, regular, classic, usual.

The default is public, because you need to provide it if you use a validator:

parser.add_option(args.sam_path, 'o', "output", "The output SAM file path.", seqan3::option_spec::DEFAULT, seqan3::output_file_validator{seqan3::output_file_open_options::create_new, {"sam"}});

Maybe we could use something like standard (which is a synonym of default) or even normal or general.

We usually use adjectives or nouns in enums, so defaulted would be the expecption...

New idea: option_spec has "require" semantics instead of "visible" semantics.

The enum names of seqan3::option_spec were changed to lower case:

seqan3::option_spec::DEFAULT is replaced by ~~seqan3::option_spec::optional~~ seqan3::option_spec::standard
(default-visible and non-required).

seqan3::option_spec::REQUIRED is replaced by seqan3::option_spec::required
(default-visible and required).

seqan3::option_spec::ADVANCED is replaced by seqan3::option_spec::advanced
(advanced-visible and non-required).

seqan3::option_spec::HIDDEN is replaced by seqan3::option_spec::hidden
(non-visible and non-required).

smehringer · 2020-12-14T11:46:40Z

Resolution 14-12-2020:

We will name it seqan3::option_spec::standard for the default case.

smehringer · 2020-12-15T06:52:04Z

@rrahn I included the namechange and updated the documentation :)

rrahn

just a minor suggestion for improvement

doc/tutorial/argument_parser/index.md

Co-authored-by: rrahn <[email protected]>

smehringer requested review from a team and simonsasse and removed request for a team November 26, 2020 15:40

simonsasse approved these changes Nov 30, 2020

View reviewed changes

eseiler reviewed Nov 30, 2020

View reviewed changes

include/seqan3/argument_parser/auxiliary.hpp Outdated Show resolved Hide resolved

[MISC] Change seqan3::option_spec enum names to lower case.

6d786e9

smehringer force-pushed the argument_parser_enum_names branch from 1630b92 to 6d786e9 Compare December 2, 2020 13:58

smehringer requested a review from rrahn December 2, 2020 14:00

rrahn requested changes Dec 5, 2020

View reviewed changes

apply requested changes.

db17ba3

smehringer requested a review from rrahn December 7, 2020 12:24

Change defaulted->standard.

d502d00

rrahn approved these changes Dec 17, 2020

View reviewed changes

doc/tutorial/argument_parser/index.md Outdated Show resolved Hide resolved

Update doc/tutorial/argument_parser/index.md

3226f4d

Co-authored-by: rrahn <[email protected]>

smehringer merged commit bd69c80 into seqan:master Dec 17, 2020

smehringer deleted the argument_parser_enum_names branch January 5, 2021 17:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MISC] Change seqan3::option_spec enum names to lower case. #2285

[MISC] Change seqan3::option_spec enum names to lower case. #2285

smehringer commented Nov 26, 2020 •

edited

Loading

codecov bot commented Nov 26, 2020 •

edited

Loading

simonsasse left a comment

smehringer commented Dec 2, 2020

rrahn left a comment

rrahn Dec 5, 2020

smehringer Dec 7, 2020 •

edited

Loading

rrahn Dec 7, 2020

smehringer Dec 7, 2020 •

edited

Loading

eseiler Dec 7, 2020 •

edited

Loading

marehr Dec 8, 2020 •

edited

Loading

smehringer commented Dec 14, 2020

smehringer commented Dec 15, 2020

rrahn left a comment

[MISC] Change seqan3::option_spec enum names to lower case. #2285

[MISC] Change seqan3::option_spec enum names to lower case. #2285

Conversation

smehringer commented Nov 26, 2020 • edited Loading

codecov bot commented Nov 26, 2020 • edited Loading

Codecov Report

simonsasse left a comment

Choose a reason for hiding this comment

smehringer commented Dec 2, 2020

rrahn left a comment

Choose a reason for hiding this comment

rrahn Dec 5, 2020

Choose a reason for hiding this comment

smehringer Dec 7, 2020 • edited Loading

Choose a reason for hiding this comment

rrahn Dec 7, 2020

Choose a reason for hiding this comment

smehringer Dec 7, 2020 • edited Loading

Choose a reason for hiding this comment

eseiler Dec 7, 2020 • edited Loading

Choose a reason for hiding this comment

marehr Dec 8, 2020 • edited Loading

Choose a reason for hiding this comment

smehringer commented Dec 14, 2020

smehringer commented Dec 15, 2020

rrahn left a comment

Choose a reason for hiding this comment

smehringer commented Nov 26, 2020 •

edited

Loading

codecov bot commented Nov 26, 2020 •

edited

Loading

smehringer Dec 7, 2020 •

edited

Loading

smehringer Dec 7, 2020 •

edited

Loading

eseiler Dec 7, 2020 •

edited

Loading

marehr Dec 8, 2020 •

edited

Loading