michaelgruenstaeudl / PACVr

Plastome Assembly Coverage Visualization in R
Other
3 stars 4 forks source link

`regionsCheck` and `syntenyLineType` as single parameter & `PACVr.verboseInformation` update #25

Closed alephnull7 closed 5 months ago

alephnull7 commented 5 months ago

The regionsCheck and syntenyLineType have been combined into regionsCheck, and resembles the previous syntenyLineType parameter. As hinted at, regionsCheck values of 1 and 2 correspond to the values previously used for syntenyLineType - 1 produces ribbon lines and 2 produces solid lines, with the choice to analyze regions implicit in these options. The value 0 for regionsCheck just performs the region analysis, without any synteny line. When I initially created the new regionsCheck parameter I imagined that there might be more broad uses for a synteny line, but any expansion of analysis or visualization choices can probably be accomplished by encoding new values available to regionsCheck, or a more general parameter name that would replace it.

The exception that occurred within writeTables appears to be an artifact of the function read.gbSampleName that I created to replace the corresponding genbankr function. During our initial testing of these replacement functions, I do not believe the use case of verbose = TRUE was tested for, and this issue was missed. In my original read.gbSampleName, the VERSION of the GenBank data was assigned to genome_name, and ACCESSION was assigned to sample_name. As I am more familiar with the properties of the data, and as you can probably see, that assignment is the opposite of their generally accepted meanings, and of their use in the GenomicRanges functions. That minor change has been applied.

Since PACVr.verboseInformation at minimum requires regions analysis to be performed to produce any output, a regionsCheck value corresponding to this is needed for it to be invoked. I did consider having the function be called unconditionally, but I felt that would currently be confusing to the user - "why don't I have any files?!!". Of course, if PACVr.verboseInformation expands to other use cases, the surrounding code can be changed to have the function be called unconditionally. I also entertained the idea of folding verbose into regionsCheck, but this seemed like an obtuse solution since the number of combinations encoded in regionsCheck would not scale well. As of now, if the parameter for regionsCheck is not an understood value (currently 0, 1, or 2) and verbose = TRUE, a log_warn is produced, indicating the values of regionsCheck for which verbose output can be produced.

For testing, I expanded to include end-to-end testing, utilizing the examples included in README.md. Hopefully, I can work toward having those files in inst/extdata/README_USAGE be embedded in the README.md or through a GitHub action be added to the README.md to reduce duplication. There does exist a GitHub-specific solution to embed code as detailed here but relies on permanent links, and as such, would not automatically update the code embedded if the files change. Edit: I may be incorrect in the ability of the above solution to reference the most updated version of the file, and I will pursue that technique in the future.