Closed wvdtoorn closed 4 years ago
Why is seqan3::views::to (seqan3/range/views/to.hpp) not included in seqan3/range/views/all.hpp? I was confused about this.
Good catch! This was not intentional. The all.hpp
header should always include every (non-detail) subheader in its module.
As discussed in last week's iteration planning, I am closing this issue and opening a separate one for the seqan3 Cookbook
Improvements to the tutorial, per section.
Setup
Quick Setup (using CMake)
[x] 0.0.1 Directory Structure
The directory should look like this:
Tutorial
[x] 1.0.1 Name/Number all the exercises and assignments in the tutorial in a consistent way.
In the current state it's hard to keep well-arranged notes on where exactly that usefull information is explained / snippet can be found.
[ ] 1.0.2 Section with common usecases code.
I think it would be very useful to include a section of more basic snippets to the How-To. A listing of common use cases helps to quickly see some of the things SeqAn3 can do. The structure of the tutorial makes that is it hard to obtain such an overview (different pages, lot's of scrolling).
Remarks such as "For this, have a look at this section of the tutorial/at this submodule" and hyperlinks to the tutorial should be included. This improves using the tutorial as documentation for users with a moderate knowledge of the library (not currently doing the tutorial, but having a question).
[ ] 1.0.3 Include pointers/tips/information on which headers to use or take a look at.
E.g. as in "Exercise: Reading a FASTQ file" from Sequence file I/O, Reading a sequence file, Reading records:
"Note: you include the seqan3::debug_stream with the following header
#include <seqan3/core/debug_stream.hpp>
"Having such remarks in the task helps with learning where everything is defined.
First steps
Align two sequences
[x] 1.1.1 Be more explicit about header includes.
In the section above, the user has been introduced to Modules in SeqAn3, i.e. how to include the needed headers. I feel that knowing which headers to include for your code is a big part of learning to work with SeqAn3 as well.
seqan3::align_cfg::edit
andseqan3::align_cfg::result
are used in the snippet, but I was confused by how that could work without the proper includes. I would suggest to be more explicit on the includes here. Either by:all.hpp
-headers as discussed in the previous sectionalignment/pairwise/align_pairwise.hpp
-header includes thealignment/configuration/all.hpp
, which in turn provides the neededseqan3::align_cfg::edit
andseqan3::align_cfg::result
functions.@seqan/core what is preferred?
Also, see 1.0.3.
--> Usage of all.hpp headers is preferred.
Parsing command line arguments
Assignment 2Exercise: Fun with views I
[x] 1.2.1 Solution should include calling
initialise_argument_parser()
.C++ Concepts
Exercise: Static polymorphism with alphabets II
[x] 1.3.1 Change wording in objective.
Change to: Adapt your previous solution to handle nucleotides differently from the rest. For nucleotides it should print both the value and its complement.
Alphabets
Excercise: GC content of a sequence
[x] 1.4.1 Views are used but were not introduced yet.
The solution to this exercise converts the input to a dna5 sequence using:
I would suggest introducing views before this exercise. Or, at least introduce the functions that are needed here and refer to the introduction to views in the Ranges section for a more in depth explanation.
**--> Change solution to contain a simple for loop using assignments from char, mention views solution and that it will be addressed later***
Ranges
Exercise: Fun with views I
[x] 1.5.1 Solution snippet is not executable.
The current solution does not contain a main function. For someone that is not very familiar with C++ that may be easily overlooked, especially as the resulting error messages aren't too explicit.
When adding main, also include the needed headers.
Exercise: Fun with views II
[x] 1.5.2 Explanation in solution is not clear.
In the last sentence of the solution it states: "This would have prevented modelling the contiguous_range as well – if it hadn't been already by the filter – because values are created on-demand and are not stored in memory at all. " It would be good to explain this a bit further. I don't think that, with the provided level of introduction to views and ranges, this is sufficient to understand what is meant.
Exercise: Fun with views III
[x] 1.5.3 Use the argument parser.
Although it is not the main objective of this section/exercise, I would suggest to use the argument parser here too. It has already been introduced, and the more practice the better. Alternatively, make it into an "optional:" and provide solutions for both options (e.g. as in "Exercise: Storing records in a std::vector" from Sequence file I/O, Reading a sequence file, The record type).
[x] 1.5.4 Inlcude a remark on including
seqan3::views::to
.In the section "Modules in SeqAn3" it is explained that including the
all.hpp
-header of a (sub)module isn't the most effective way to do things, but it works. Includingseqan3/range/views/all.hpp
however does not include the functionseqan3::views::to
that is needed here. This needs an explanation.@seqan3/core : Why is
seqan3::views::to
(seqan3/range/views/to.hpp
) not included inseqan3/range/views/all.hpp
? I was confused about this.Sequence file I/O
[x] 1.6.1 Remove "TODO" in warning.
The text in the warning box seems to include the required info on bz2. Remove the TODO sentence from the tutorial or, if it is essential to include that something regarding bz2 is work in progress, explain/specify this further.
Sequence file formats/File extensions
[x] 1.6.2 Inconsistent naming file extension member.
In the text it says "You can access the valid file extension via the
file_extension
member variable in a format:". In the snippet belowfile_extensions
is used, with an '-s'.Reading a sequence file/Reading records
[x] 1.6.3 Remove
using seqan3::get;
from snippet.I believe that writing everything in full helps with learning where everything can be found.
Sequence files as views/Applying a filter to a file
[x] 1.6.4 In snippet: ‘accumulate’ is not a member of ‘ranges’
Either state what header to include to make this work, or use std::accumulate.
--> Use std::ranges::accumulate
Sequence files as views/Reading paired-end reads
[x] 1.6.5 Inlcude a remark on including seqan3::views::move.
Similar to 1.5.4, explain why
seqan3::views::move
(seqan3/range/views/move.hpp
) is not included when includingseqan3/range/views/all.hpp
?Computing pairwise alignments
[x] 1.7.0 Fix width of image "align_transcript.png"
[x] 1.7.1 Grammar error.
In the text below the first snippet, second paragraph, remove one instance of the word "sequence".
[x] 1.7.2 Use consistent naming of variables.
In the text below the first snippet, second paragraph, the DNA sequences are called
first_seq
andsecond_seq
. In the snippet they are calleds2
ands2
.[x]
1.7.3 In snippet: include an alternative implemenation usingviews
.Good to practice.A listing of both these alternatives would also be very well suited for 1.0.2.After further consideration, does not really make sense to use a view here.Assignment 2
[x] 1.7.4 Improve wording of task description.
Currently, it is hard to understand what the exercise is exactly without looking at the solution. Add more details.
Assignment 3
[x] 1.7.5 Use the same variable in task description and solution.
The second amino acid sequence in the exercise differs from the one that is used in the solution.
[x] 1.7.6 In solution:
#include <utility>
.Why is
utility
imported? This is not explained, nor consistently done in similar solutions. Either remove or explain the usage.[x] 1.7.7 Be more explicit on header includes.
Header file usage is confusing here. Add note that the header file
seqan3/alignment/cofiguration/all.hpp
does not need to be included as it is already included viaseqan3/alignment/pairwise/align_pairwise.hpp
. See 1.1.1 (and 1.0.3).Refine edit distance
[x] 1.7.8 Change name of assignment.
This is the second "Assignment 5" in this tutorial.