Open marehr opened 5 months ago
The latest updates on your projects. Learn more about Vercel for Git ↗︎
Name | Status | Preview | Updated (UTC) |
---|---|---|---|
seqan3 | ❌ Failed (Inspect) | Jan 30, 2024 4:16pm |
Attention: 2 lines
in your changes are missing coverage. Please review.
Comparison is base (
45897e6
) 98.17% compared to head (1c3a75b
) 98.19%.
Files | Patch % | Lines |
---|---|---|
...ude/seqan3/core/debug_stream/debug_stream_type.hpp | 88.88% | 2 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Thanks a lot!
Can you also add a to-do list with stuff that still must/needs/should be done regarding the code?
Thanks a lot!
Can you also add a to-do list with stuff that still must/needs/should be done regarding the code?
Not sure if you meant it like this:
std::same_as
instead of std::is_same_v
in require checksdefault_printer.hpp
into multiple header
printer_fwd.hpp
: Put printer forward declarations into own forward headerprinter_order.hpp
: Put printer_order
and printable
into it (you might want to use seqan3::type_list::find_index`)std_printer.hpp
: Put std_printer
struct def intointegral_printer.hpp
: Put integral_printer
struct def intodefault_printer.hpp
: Keep default_printer
structdebug_stream
should be accessible by (public) api (removes friend decls)seqan3::shape
should be printed
https://github.com/seqan/seqan3/blob/f97db8376f629e7555c0ab7c76db9a0ac8512eeb/test/snippet/search/kmer_index/shape.cpp#L12-L26operator<<
by printer
structs (all #if 1
sections)seqan3::printable
seqan3::printer_order
and all public api printer_for_t
, printers_for_t
(this one is basically just there for debugging purposes as it gives a list of printers that can print that type), is_printable
, print
printer
struct (e.g. alphabet_printer
) is able to print the instance of the type; to avoid unnecessary includes.seqan3::alphabet_printer
and seqan3::default_printer
behave the same, calling wise. The idea would be, one test instance where just seqan3::alphabet_printer
is included without any other include dependency, and one test where all other printers are included (basically seqan3/debugstream/all.hpp
) with the same "TypedTest".Do you see more?
This PR will resolve seqan/product_backlog#63.
This issue is a long-standing open to-do of mine. I hope that you can take it over and push it over the finishing line. The state of the current PR is just a draft of an idea.
I'll comment on multiple code locations to point out the advantages of the new design.
The old design
The major idea of the design is due to the following observation:
Just to give a small example (the more down, the more specialized):
NOTE: that using concepts as overload resolution always uses a partially ordered set, which can be depicted by as a Hasse Diagram, and by using the except clauses via
requires
we give it a total order.The new design
Before anything, note that the new design does not break any existing code. As existing
seqan3::debugstream <<
overloads, still take place in overload resolution.The idea is simple:
printer<type>
either has a function object / lambdaprint
or not; depending on whether theprinter
can print thattype
or not (implemented by template specialization)printer
in other printer's, if we know that only that overload should be used,So put together: For a given type, ask every printer in order whether it can print that type and the first one to answer yes, is the selected printer.
[1] If all overloads do not work, use
std::ostream
https://github.com/seqan/seqan3/blob/6b681fb2eae5ab2997d293e99fc6a7f869a20316/include/seqan3/core/debug_stream/debug_stream_type.hpp#L242-L247 [2] Use this for allstd::ranges::input_range
s except if type is something likestd::filesystem
(type == range_value_t) orchar *
https://github.com/seqan/seqan3/blob/6b681fb2eae5ab2997d293e99fc6a7f869a20316/include/seqan3/core/debug_stream/range.hpp#L96-L98 https://github.com/seqan/seqan3/blob/6b681fb2eae5ab2997d293e99fc6a7f869a20316/include/seqan3/core/debug_stream/range.hpp#L38-L45 [3] Same as [2] where value_type is an alphabet but only if the alphabet is not anunsigned int
(this condition has no test case?!) https://github.com/seqan/seqan3/blob/6b681fb2eae5ab2997d293e99fc6a7f869a20316/include/seqan3/core/debug_stream/range.hpp#L138-L141 [4] Use this for allstd::tuple
-like types except if it is astd::ranges::input_range
(what is a tuple and ranges at the same time?!) and anseqan3::alphabet
(basicallyseqan3::alphabet_tuple_base
derived types) https://github.com/seqan/seqan3/blob/6b681fb2eae5ab2997d293e99fc6a7f869a20316/include/seqan3/core/debug_stream/tuple.hpp#L53-L56 [5] Use this for allseqan3::alphabet
s except if it can be printed bystd::cout
(likechar
) https://github.com/seqan/seqan3/blob/6b681fb2eae5ab2997d293e99fc6a7f869a20316/include/seqan3/alphabet/detail/debug_stream_alphabet.hpp#L30-L32 [6] Type must beseqan3::semialphabet
andseqan3::mask
https://github.com/seqan/seqan3/blob/6b681fb2eae5ab2997d293e99fc6a7f869a20316/include/seqan3/alphabet/detail/debug_stream_alphabet.hpp#L46-L48