[FEATURE] Alignment Configuration: put align_cfg::debug to align_cfg::detail::debug

[Irallia]

Resolves seqan/product_backlog#181

Blocked by #2033

[Irallia]

I get some documentation errors:

/Users/lydia/Repos/seqan3/include/seqan3/alignment/configuration/detail.hpp:29: warning: unable to resolve reference to 'seqan3::align_cfg::detail::debug' for \ref command
/Users/lydia/Repos/seqan3/include/seqan3/alignment/all.hpp:92: warning: unable to resolve reference to 'seqan3::align_cfg::detail::debug' for \ref command
/Users/lydia/Repos/seqan3/include/seqan3/alignment/configuration/detail.hpp:29: warning: unable to resolve reference to 'seqan3::align_cfg::detail::debug' for \ref command

Does someone know how to solve them? Do I need to add a config detail struct?

[marehr]

You have unrelated commits (see https://github.com/seqan/seqan3/pull/2020/commits) in your history, could you remove them?

git checkout feature/alignment/configuration/put_align_cfg_debug_to_align_cfg_detail_debug
git rebase -i upstream/release-3.0.2
# remove the commits that creeped in.
# and then force push
git push origin feature/alignment/configuration/put_align_cfg_debug_to_align_cfg_detail_debug -f

(This is not my complete review, please re-request review when done.)

[marehr]

@seqan/core Do we want to add a CHANGELOG entry for moved entities from seqan3::align_cfg namespace to seqan3::align_cfg::detail?

[smehringer]

hm should the user know about the debug functionality at all? If not, I wouldn't put it into the changelog

[Irallia]

Yesterday I discussed this with @joergi-w because he uses many of our detailed functions for his app. The debug_stream, for example, is very practical.

[marehr]

The debug_stream is public API and should get a CHANGELOG entry if it changes API.

seqan3/include/seqan3/alignment/configuration/align_config_debug.hpp

Lines 19 to 38 in c24c911

	namespace seqan3::align_cfg
	{
	//!\cond DEV
	/*!\brief Configuration element for debugging the alignment algorithm.
	* \ingroup alignment_configuration
	*
	* \details
	*
	* Using this configuration allows to output the alignment matrices from the DP algorithm using the
	* returned seqan3::alignment_result.
	* The score matrix is always accessible, while the trace matrix can only be computed if an alignment was
	* requested via the seqan3::align_cfg::result configuration.
	*
	* \note This configuration is only useful for debugging purposes as it can have a significant impact on the
	* performance.
	*/
	inline constexpr detail::debug_mode<std::integral_constant<detail::align_config_id,
	detail::align_config_id::debug>> debug{};
	//!\endcond
	} // namespace seqan3::align_cfg

seqan3::align_cfg::debug is semi-public and was only used within tests. If it is the only change, I wouldn't mention it. (I only cursorily looked over the PR)

EDIT:// after looking at the PR, I wouldn't mention this.

[rrahn]

No the debug thing is developer API only. And as far as I know it was also only documented like so. Which means the regular user does not know about it. That's why we decided to put it into a specific detail sub namespace.

Yesterday I discussed this with @joergi-w because he uses many of our detailed functions for his app. The debug_stream, for example, is very practical.

That is the responsibility of the app developer. We don't give any guarantees about the detail namespace. That can change at any time without notice.

[marehr]

@Irallia I pushed directly i your branch to fix the last things. Can you rebase and re-order the history?

The name-clash commits changes should be before adding the namespace.

[marehr]

(See comment before)

[codecov]

Codecov Report

Merging #2020 into release-3.0.2 will not change coverage.
The diff coverage is n/a.

@@              Coverage Diff               @@
##           release-3.0.2    #2020   +/-   ##
==============================================
  Coverage          97.89%   97.89%           
==============================================
  Files                263      263           
  Lines               9879     9879           
==============================================
  Hits                9671     9671           
  Misses               208      208           

Impacted Files	Coverage Δ
.../seqan3/alignment/pairwise/alignment_algorithm.hpp	98.50% <ø> (ø)
...qan3/alignment/pairwise/alignment_configurator.hpp	100.00% <ø> (ø)
...ude/seqan3/alignment/pairwise/alignment_result.hpp	100.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 2ce0448...60eb66f. Read the comment docs.

[marehr]

@Irallia can you rebase?

[marehr]

thank you :)

[smehringer]

LGTM :)