[FEATURE] Search dynamic mode config #1446
Conversation
rrahn
force-pushed
the
feature/search_dynamic_mode_config
branch
from
c467ff9
to
e3e776f
Compare
Codecov Report
@@ Coverage Diff @@
## master #1446 +/- ##
==========================================
+ Coverage 97.66% 97.66% +<.01%
==========================================
Files 236 238 +2
Lines 8988 9011 +23
==========================================
+ Hits 8778 8801 +23
Misses 210 210
Continue to review full report at Codecov.
|
test/unit/search/search_test.cpp
Outdated
| @@ -300,6 +300,13 @@ TYPED_TEST(search_test, search_strategy_all) | |||
| configuration const cfg = max_error{total{1}} | mode{all}; | |||
| EXPECT_EQ(uniquify(search("ACGT"_dna4, this->index, cfg)), (hits_result_t{0, 1, 4, 5, 8, 9})); | |||
| } | |||
|
|
|||
| { // Test with dynamic mode. | |||
| mode dynamic_mode{}; | |||
There was a problem hiding this comment.
If I remember correctly, in tests we now have to specify seqan3:: namespace before all types and functions of seqan3?
There was a problem hiding this comment.
yes, but I won't have this refactoring done for this PR, since it is about a feature. The other thing must be done separately.
rrahn
force-pushed
the
feature/search_dynamic_mode_config
branch
from
e3e776f
to
5dd977b
Compare
|
@joergi-w I had to make a force push after I rebased onto master. So I also incorporated the changes already into the respective commits. I am sorry if this causes some inconvenience. |
rrahn
force-pushed
the
feature/search_dynamic_mode_config
branch
from
5dd977b
to
7072a96
Compare
|
@smehringer polite ping |
|
I did not manage to review this last week and since this is not an urgent issue and we need to redesign the config, doesn't it make more since to refactor first and then apply this change? Otherwise we need to refactor this code again? |
Since it is not clear when we are going to refactor it, this should be already added. The other point is that the mechanics will be the same, just different class names are used. So I don't see that this can't be incrementally adapted later? |
|
I thought we decided to postpone this altogether? |
I think this is a very independent PR. The search result range should wait until after the release because it involves more stuff. But anyway, we could also put that in after the release. I don't care. In general, I was trying to make a point why we should not postpone it until we start refactoring the things. I would like to do it first, when we arrive at an actual use case. But that is independent of the mechanics of this PR. |
eseiler
changed the title
This type can be used to select the runtime configuration for some configuration elements.
Imports the function call operator for the given invocable types. This can be used for example in combination with the visitor pattern over a std::variant.
Adds a dynamic state to the mode configuration of the search algorithm. This mode can be modified at runtime through assignment of an associated mode option.
Enables the dynamic mode configuration within the search algorithm by transforming it into a static configuration.
rrahn
force-pushed
the
feature/search_dynamic_mode_config
branch
from
7072a96
to
9f83164
Compare
|
Will be tackled as part of the next iteration with the refined configuration naming. |
Resolves #1436