search

michaelgruenstaeudl / PACVr

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

Refactor of `PACVr.complete`; decoding `IRCheck` as a single variable; handle use of `note` qualifier for IR name #27

Closed alephnull7 closed 5 months ago

alephnull7 commented 5 months ago

In an attempt to reduce the complexity present in PACVr.complete, code responsible for producing an individual variable is present as its own function within PACVr.R. For similar reasons, the bulk of the code for PACVr.verboseInformation has been moved to helpers.R. Related to this goal of better clarity, the parameter of PACVR.complete IRCheck is decoded into a single variable, which can then be passed accordingly and have its elements be accessed as needed.

To address variances in GenBank data, specifically regarding which qualifier is used to detail inverted repeat names, additional checks within checkFeatureQualifiers are performed.

michaelgruenstaeudl commented 5 months ago

In an attempt to reduce the complexity present in PACVr.complete, code responsible for producing an individual variable is present as its own function within PACVr.R. For similar reasons, the bulk of the code for PACVr.verboseInformation has been moved to helpers.R. Related to this goal of better clarity, the parameter of PACVR.complete IRCheck is decoded into a single variable, which can then be passed accordingly and have its elements be accessed as needed.

To address variances in GenBank data, specifically regarding which qualifier is used to detail inverted repeat names, additional checks within checkFeatureQualifiers are performed.

Hi Greg, yes, these are very sensible changes! Great work!

Continuing this goal of providing clarification, can you please rename any variable and function called "verbose" or "verboseInformation()" with "printCovStats()" (or words to that effect), which is what it actually does. Similarly, "writeTables()" should be renamed to "printCovValsAsTable()" (or words to that effect).

Also, it would be fantastic if these coverageStats could be calculated irrespective of analysisSpecs$isIRCheck. After all, the IRCheck only has a relevance for step 3 of verboseInformation() in helpers.R, where writeTables() in its current implementation expects the exact location information of the IRs. What is needed is a second, simpler version of writeTables() that does not operate on the four regions of a quadripartite genome but simply on the entire undivided genome.

Thank you and best, Michael