Skip to content

Commit

Permalink
[BUG] extend_right/left() needs char const * overload (#1588)
Browse files Browse the repository at this point in the history
* [FIX] extend_right() needs `char const *` overload

* [TEST] add tests for issue #1473

* [FIX] missing initialisation added
  • Loading branch information
Irallia committed Apr 20, 2020
1 parent b2cdc3e commit d921134
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 9 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ Note that 3.1.0 will be the first API stable release and interfaces in this rele

## Notable Bug-fixes

#### Search

* The `seqan3::fm_index_cursor::extend_right()`, `seqan3::bi_fm_index_cursor::extend_right()` and
`seqan3::bi_fm_index_cursor::extend_left()` functions handle c-style strings without including the null character
([\#1588](https://github.com/seqan/seqan3/pull/1588)).

# 3.0.1

Note that 3.1.0 will be the first API stable release and interfaces in this release might still change.
Expand Down
25 changes: 23 additions & 2 deletions include/seqan3/search/fm_index/bi_fm_index_cursor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include <seqan3/alphabet/adaptation/char.hpp>
#include <seqan3/alphabet/adaptation/uint.hpp>
#include <seqan3/alphabet/concept.hpp>
#include <seqan3/core/type_traits/range.hpp>
#include <seqan3/range/views/slice.hpp>
#include <seqan3/search/fm_index/bi_fm_index.hpp>
Expand Down Expand Up @@ -118,9 +119,9 @@ class bi_fm_index_cursor
// need to store it twice. Once the cursor is switched, the information becomes invalid anyway.

//!\brief Left suffix array interval of the parent node.
size_type parent_lb;
size_type parent_lb{};
//!\brief Left suffix array interval of the parent node.
size_type parent_rb;
size_type parent_rb{};
//!\brief Label of the last edge moved down. Needed for cycle_back() or cycle_front().
sdsl_char_type _last_char;
//\}
Expand Down Expand Up @@ -441,6 +442,16 @@ class bi_fm_index_cursor
return false;
}

//!\overload
template <typename char_type>
//!\cond
requires seqan3::detail::is_char_adaptation_v<char_type>
//!\endcond
bool extend_right(char_type const * cstring) noexcept
{
return extend_right(std::basic_string_view<char_type>{cstring});
}

/*!\brief Tries to extend the query by the character `c` to the left.
* \tparam char_t Type of the character needs to be convertible to the character type `char_type` of the index.
* \param[in] c Character to extend the query with to the left.
Expand Down Expand Up @@ -486,6 +497,16 @@ class bi_fm_index_cursor
return false;
}

//!\overload
template <typename char_type>
//!\cond
requires seqan3::detail::is_char_adaptation_v<char_type>
//!\endcond
bool extend_left(char_type const * cstring) noexcept
{
return extend_left(std::basic_string_view<char_type>{cstring});
}

/*!\brief Tries to extend the query by `seq` to the right.
* \tparam seq_t The type of range of the sequence to search; must model std::ranges::forward_range.
* \param[in] seq Sequence to extend the query with to the right.
Expand Down
21 changes: 16 additions & 5 deletions include/seqan3/search/fm_index/fm_index_cursor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <range/v3/view/slice.hpp>

#include <seqan3/alphabet/adaptation/char.hpp>
#include <seqan3/alphabet/concept.hpp>
#include <seqan3/core/type_traits/range.hpp>
#include <seqan3/range/views/slice.hpp>
Expand Down Expand Up @@ -86,15 +87,15 @@ class fm_index_cursor
//!\}

//!\brief Underlying FM index.
index_type const * index;
index_type const * index{nullptr};
//!\brief Left suffix array interval of the parent node. Needed for cycle_back().
size_type parent_lb;
size_type parent_lb{};
//!\brief Right suffix array interval of the parent node. Needed for cycle_back().
size_type parent_rb;
size_type parent_rb{};
//!\brief Underlying index from the SDSL.
node_type node;
node_type node{};
//!\brief Alphabet size of the index without delimiters
sdsl_sigma_type sigma;
sdsl_sigma_type sigma{};

template <typename _index_t>
friend class bi_fm_index_cursor;
Expand Down Expand Up @@ -290,6 +291,16 @@ class fm_index_cursor
return false;
}

//!\overload
template <typename char_type>
//!\cond
requires detail::is_char_adaptation_v<char_type>
//!\endcond
bool extend_right(char_type const * cstring) noexcept
{
return extend_right(std::basic_string_view<char_type>{cstring});
}

/*!\brief Tries to extend the query by `seq` to the right.
* \tparam seq_t The type of range of the sequence to search; must model std::ranges::forward_range.
* \param[in] seq Sequence to extend the query with to the right.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,5 +222,38 @@ TYPED_TEST_P(bi_fm_index_cursor_collection_test, to_rev_cursor)
}
}

TYPED_TEST_P(bi_fm_index_cursor_collection_test, extend_const_char_pointer)
{
using alphabet_type = typename TestFixture::alphabet_type;

// test case for https://github.com/seqan/seqan3/issues/1473
if constexpr (std::is_same<alphabet_type, char>::value)
{
typename TypeParam::index_type fm{this->text_col1}; // {"AACGATCGGA", "AACGATCGGA"}
char const * cg = "CG";

// extend_right()
{
TypeParam it1 = TypeParam(fm);
TypeParam it2 = TypeParam(fm);

it1.extend_right(cg);
it2.extend_right(seqan3::views::slice(this->text1, 1, 3)); // "CG"

EXPECT_TRUE(std::ranges::equal(it1.locate(), it2.locate()));
}
// extend_left()
{
auto it1 = TypeParam(fm);
auto it2 = TypeParam(fm);

it1.extend_left(cg);
it2.extend_right(seqan3::views::slice(this->text1, 1, 3)); // "CG"

EXPECT_TRUE(std::ranges::equal(it1.locate(), it2.locate()));
}
}
}

REGISTER_TYPED_TEST_SUITE_P(bi_fm_index_cursor_collection_test, cursor, extend, extend_char, extend_range,
extend_and_cycle, extend_range_and_cycle, to_fwd_cursor, to_rev_cursor);
extend_and_cycle, extend_range_and_cycle, to_fwd_cursor, to_rev_cursor, extend_const_char_pointer);
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,27 @@ TYPED_TEST_P(fm_index_cursor_collection_test, lazy_locate)
EXPECT_TRUE(std::ranges::equal(it.locate(), it.lazy_locate()));
}

TYPED_TEST_P(fm_index_cursor_collection_test, extend_const_char_pointer)
{
using alphabet_type = typename TestFixture::alphabet_type;

// test case for https://github.com/seqan/seqan3/issues/1473
if constexpr (std::is_same<alphabet_type, char>::value)
{
typename TypeParam::index_type fm{this->text_col1}; // {"ACGACG", "ACGACG"}
char const * cg = "CG";

// extend_right()
TypeParam it1 = TypeParam(fm);
TypeParam it2 = TypeParam(fm);

it1.extend_right(cg);
it2.extend_right(seqan3::views::slice(this->text1, 1, 3)); // "CG"

EXPECT_TRUE(std::ranges::equal(it1.locate(), it2.locate())); // [(0,1),(0,4),(1,4),(1,1)]
}
}

TYPED_TEST_P(fm_index_cursor_collection_test, concept_check)
{
EXPECT_TRUE(seqan3::fm_index_cursor_specialisation<TypeParam>);
Expand All @@ -329,4 +350,4 @@ TYPED_TEST_P(fm_index_cursor_collection_test, concept_check)
REGISTER_TYPED_TEST_SUITE_P(fm_index_cursor_collection_test, ctr, begin, extend_right_range,
extend_right_range_empty_text, extend_right_char, extend_right_range_and_cycle,
extend_right_char_and_cycle, extend_right_and_cycle, query, last_rank, incomplete_alphabet,
lazy_locate, concept_check);
lazy_locate, extend_const_char_pointer, concept_check);

0 comments on commit d921134

Please sign in to comment.