remove #include<meta/meta.hpp> #2583

marehr · 2021-04-30T21:08:36Z

vercel · 2021-04-30T21:08:41Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/seqan/seqan3/EZHKWA3yyZraBuoN9pGGTHdgnnqZ
✅ Preview: https://seqan3-git-fork-marehr-removemeta-seqan.vercel.app

codecov · 2021-04-30T21:41:38Z

Codecov Report

Merging #2583 (08578f0) into master (fcad6ef) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2583   +/-   ##
=======================================
  Coverage   98.23%   98.24%           
=======================================
  Files         269      270    +1     
  Lines       10561    10570    +9     
=======================================
+ Hits        10375    10384    +9     
  Misses        186      186

Impacted Files	Coverage Δ
...clude/seqan3/alignment/pairwise/align_pairwise.hpp	`100.00% <ø> (ø)`
.../seqan3/alphabet/composite/alphabet_tuple_base.hpp	`100.00% <100.00%> (ø)`
...ude/seqan3/alphabet/composite/alphabet_variant.hpp	`100.00% <100.00%> (ø)`
include/seqan3/utility/type_list/type_list.hpp	`100.00% <100.00%> (ø)`
include/seqan3/utility/views/pairwise_combine.hpp	`100.00% <0.00%> (ø)`

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 fcad6ef...08578f0. Read the comment docs.

Irallia

LGFM, only two style things. 💅

include/seqan3/utility/tuple/concept.hpp

test/unit/utility/type_list/type_list_traits_test.cpp

eseiler

I need to wrap my head around this a bit more, but I saw one thing

eseiler · 2021-05-03T12:00:12Z

include/seqan3/alphabet/composite/alphabet_tuple_base.hpp

    static constexpr bool is_unique_component =
        is_component<type> &&
-        (meta::find_index<component_list, type>::value == meta::reverse_find_index<component_list, type>::value);
+        (seqan3::list_traits::size<decltype(seqan3::list_traits::detail::filter_out<type>(component_list{}))>
+         == sizeof...(component_types) - 1);


Suggested change

static constexpr bool is_unique_component =

is_component<type> &&

(meta::find_index<component_list, type>::value == meta::reverse_find_index<component_list, type>::value);

(seqan3::list_traits::size<decltype(seqan3::list_traits::detail::filter_out<type>(component_list{}))>

== sizeof...(component_types) - 1);

static constexpr bool is_unique_component = seqan3::list_traits::count<type, component_list> == 1;

?

I'm stupid xD Thank you, I was blended by the reverse.

include/seqan3/alphabet/composite/alphabet_tuple_base.hpp

eseiler · 2021-05-03T13:34:58Z

include/seqan3/alphabet/composite/alphabet_tuple_base.hpp

-    static constexpr bool is_unique_component =
-        is_component<type> &&
-        (meta::find_index<component_list, type>::value == meta::reverse_find_index<component_list, type>::value);
+    static constexpr bool is_unique_component = (seqan3::list_traits::count<type, component_list> == 1);


Actually does not need parenthesis because it's not a concept, but they don't hurt either; actually easier to read with ()

No hard feeling :)

eseiler · 2021-05-03T13:58:36Z

include/seqan3/utility/tuple/concept.hpp

+template <typename ...elements_t>
+inline constexpr auto all_elements_model_totally_ordered(seqan3::type_list<elements_t...>)
+    -> std::bool_constant<(std::totally_ordered<elements_t> && ... && true)>;


I think the behavior is the same

old: https://godbolt.org/z/cGxKKndPT

new: https://godbolt.org/z/dboEvMsc9

(Especially for empty lists I wasn't sure)

Do we need the && true? Because it also works without it (at least in 10 and 11).

The true makes it work for empty lists; In this case we already handle empty lists before, but it doesn't hurt here.

ty for verifying :)

eseiler · 2021-05-03T14:04:11Z

include/seqan3/utility/type_list/traits.hpp

+//!\brief A helper for seqan3::list_traits::detail::unique [recursion anchor]
+template <typename query_t>
+type_list<> filter_out(type_list<>);
+
+//!\brief A helper for seqan3::list_traits::detail::unique [recursion]
+template <typename query_t, typename head_t, typename ...pack_t>
+auto filter_out(type_list<head_t, pack_t...>)
+{
+    if constexpr(std::same_as<query_t, head_t>)
+        return filter_out<query_t>(type_list<pack_t...>{});
+    else
+        return concat(type_list<head_t>{}, filter_out<query_t>(type_list<pack_t...>{}));
+}
+
+//!\brief A replacement for meta::unique [recursion anchor]
+inline constexpr type_list<> unique(type_list<>) { return {}; }
+
+//!\brief A replacement for meta::unique [recursion]
+template <typename head_t, typename ...pack_t>
+auto unique(type_list<head_t, pack_t...>)
+{
+    return concat(type_list<head_t>{}, unique(filter_out<head_t>(type_list<pack_t...>{})));
+}
+
+//!\brief A replacement for meta::reverse [recursion anchor]
+inline constexpr type_list<> reverse(type_list<>) { return {}; }
+
+//!\brief A replacement for meta::reverse [recursion]
+template <typename head_t, typename ...pack_t>
+auto reverse(type_list<head_t, pack_t...>)
+{
+    return concat(reverse(type_list<pack_t...>{}), type_list<head_t>{});
+}
+


This should get proper documentation, like the other traits.
It may be a follow-up issue

They are all in detail:: for now to skip the documentation :) but agree should be a follow-up

eseiler · 2021-05-03T14:09:11Z

include/seqan3/utility/type_list/traits.hpp

+//!\brief A helper for seqan3::list_traits::detail::unique [recursion anchor]
+template <typename query_t>
+type_list<> filter_out(type_list<>);
+
+//!\brief A helper for seqan3::list_traits::detail::unique [recursion]
+template <typename query_t, typename head_t, typename ...pack_t>
+auto filter_out(type_list<head_t, pack_t...>)
+{
+    if constexpr(std::same_as<query_t, head_t>)
+        return filter_out<query_t>(type_list<pack_t...>{});
+    else
+        return concat(type_list<head_t>{}, filter_out<query_t>(type_list<pack_t...>{}));
+}


Maybe remove or remove_all would be a better name?

"remove query_t from the list pack_t..."
"remove all query_t from the list pack_t..."

remove (sometimes) implies that it stops after one hit, that's why remove_all might be better

"filter out" sounds a bit weird and the cpp20-me expects to pass a predicate :)

We currently have

seqan3::list_traits::count<int, list_t> (element) https://en.cppreference.com/w/cpp/algorithm/count

seqan3::list_traits::find<double, list_t> (element) https://en.cppreference.com/w/cpp/algorithm/find

seqan3::list_traits::find_if<std::is_pointer, list_t> (predicate) https://en.cppreference.com/w/cpp/algorithm/find

seqan3::list_traits::remove<double, list_t> (element) https://en.cppreference.com/w/cpp/algorithm/remove

Removes all elements satisfying specific criteria from the range [first, last) and returns a past-the-end iterator for the new end of the range.

I like that suggestion, I'll rename it remove

Because of your ingenious idea of using count == 1, I could throw away the unique and remove :)

…t_traits::detail::unique

…le count

marehr added 10 commits April 30, 2021 23:07

[TEST] remove meta/meta.hpp include

6b777ee

[MISC] remove meta/meta.hpp include

d64ec8e

[MISC] replace meta::list by seqan3::type_list

1b13961

[MISC] replace meta::front by seqan3::list_traits::front

464a51e

[MISC] replace meta::find_if by seqan3::list_traits::find_if (and ::at)

0d6d4f5

[MISC] replace meta::at_c by seqan3::list_traits::at

92d7ae6

[MISC] replace meta::in by seqan3::list_traits::contains

55f2e5e

[MISC] replace meta::find_index by seqan3::list_traits::find

12da74d

[MISC] replace meta::concat by seqan3::list_traits::concat

dd017c9

[MISC] replace meta::for_each by seqan3::detail::for_each

cc95335

marehr requested review from a team and Irallia and removed request for a team April 30, 2021 21:08

marehr force-pushed the remove_meta branch from a3fc393 to 54b3500 Compare April 30, 2021 21:41

Irallia approved these changes May 3, 2021

View reviewed changes

include/seqan3/utility/tuple/concept.hpp Show resolved Hide resolved

test/unit/utility/type_list/type_list_traits_test.cpp Outdated Show resolved Hide resolved

marehr requested review from a team and eseiler and removed request for a team May 3, 2021 11:17

eseiler requested changes May 3, 2021

View reviewed changes

marehr commented May 3, 2021

View reviewed changes

include/seqan3/alphabet/composite/alphabet_tuple_base.hpp Outdated Show resolved Hide resolved

eseiler reviewed May 3, 2021

View reviewed changes

include/seqan3/alphabet/composite/alphabet_tuple_base.hpp Outdated Show resolved Hide resolved

eseiler reviewed May 3, 2021

View reviewed changes

include/seqan3/alphabet/composite/alphabet_tuple_base.hpp Outdated Show resolved Hide resolved

marehr commented May 3, 2021

View reviewed changes

include/seqan3/alphabet/composite/alphabet_tuple_base.hpp Outdated Show resolved Hide resolved

include/seqan3/alphabet/composite/alphabet_tuple_base.hpp Outdated Show resolved Hide resolved

eseiler reviewed May 3, 2021

View reviewed changes

eseiler requested changes May 3, 2021

View reviewed changes

marehr added 3 commits May 3, 2021 20:32

[MISC] replace meta::unique / meta::reverse_find_index by seqan3::lis…

42fcd27

…t_traits::detail::unique

[TEST] use EXPECT_SAME_TYPE

7bfbab8

[MISC] replace meta::reverse by seqan3::list_traits::detail::reverse

b975662

marehr added 4 commits May 3, 2021 20:32

[MISC] replace meta::fold by fold expression

1fe2a64

[MISC] remove meta/meta.hpp include

16ab768

[MISC] replace seqan3::list alias meta::list by own implementation

aac9a8a

[MISC] replace seqan3::list_traits::detail::{unique,remove} by a simp…

08578f0

…le count

marehr force-pushed the remove_meta branch from 125250c to 08578f0 Compare May 3, 2021 18:46

marehr requested a review from eseiler May 3, 2021 21:13

eseiler approved these changes May 4, 2021

View reviewed changes

eseiler merged commit 9c9e01b into seqan:master May 4, 2021

eseiler mentioned this pull request May 4, 2021

Add snippets for seqan3::list_traits::detail::reverse seqan/product_backlog#382

Open

marehr deleted the remove_meta branch May 4, 2021 13:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove #include<meta/meta.hpp> #2583

remove #include<meta/meta.hpp> #2583

marehr commented Apr 30, 2021

vercel bot commented Apr 30, 2021 •

edited

Loading

codecov bot commented Apr 30, 2021 •

edited

Loading

Irallia left a comment

eseiler left a comment

eseiler May 3, 2021 •

edited

Loading

marehr May 3, 2021

eseiler May 3, 2021

marehr May 3, 2021

eseiler May 3, 2021

marehr May 3, 2021

marehr May 3, 2021

eseiler May 3, 2021

marehr May 3, 2021

eseiler May 3, 2021

marehr May 3, 2021 •

edited

Loading

marehr May 3, 2021

remove #include<meta/meta.hpp> #2583

remove #include<meta/meta.hpp> #2583

Conversation

marehr commented Apr 30, 2021

vercel bot commented Apr 30, 2021 • edited Loading

codecov bot commented Apr 30, 2021 • edited Loading

Codecov Report

Irallia left a comment

Choose a reason for hiding this comment

eseiler left a comment

Choose a reason for hiding this comment

eseiler May 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marehr May 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Apr 30, 2021 •

edited

Loading

codecov bot commented Apr 30, 2021 •

edited

Loading

eseiler May 3, 2021 •

edited

Loading

marehr May 3, 2021 •

edited

Loading