Further figure making and CLI updates

[imallona]

• updated CLI (user friendliness), but broke intuitions for shorthards. Kept old args rollbacked on cfec1e7
• minor fix for report generation
• add sines/lines/Shannons/norm entropies figures

Before merging:

• Get feedback from Atreya re: CLI args
• add lines/sines, genes there were already

[imallona]

results differ / tests not passing after CLI renaming, most likely breaking positional logic? or the --all-meth or headers?

Trying headers first - yep it was that.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

method/src/boost.cpp:171

• getSkipHeaderTemplate and its wrappers still query variables_map with the keys skip-header-cell, skip-header-reference, skip-header-intervals, and skip-header, but the options are now declared under canonical names like skip-header-cytosine_report and skip-header-all. With Boost.Program_options, the stored key is the first name passed to add_options, so these lookups will always miss and the functions will always fall through to the hard-coded 0 default, disabling header skipping even when the user provides the new or legacy flags. Please update these lookups to use the canonical option names that are actually registered in parseCommandLine so that --skip-header-* behaves as documented.

unsigned int getSkipHeaderTemplate(const po::variables_map &vm, const std::string &file_type) {
  if (vm.count(file_type)) {
    return vm[file_type].as<unsigned int>();
  } else if (vm.count("skip-header")) {
    return vm["skip-header"].as<unsigned int>();
  } else {
    return 0;
  }
}

unsigned int getSkipHeaderCell(const po::variables_map &vm) {
  return getSkipHeaderTemplate(vm, "skip-header-cell");
}

unsigned int getSkipHeaderReference(const po::variables_map &vm) {
  return getSkipHeaderTemplate(vm, "skip-header-reference");
}

unsigned int getSkipHeaderIntervals(const po::variables_map &vm) {
  return getSkipHeaderTemplate(vm, "skip-header-intervals");

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The option is now registered with canonical name "cytosine_report" and aliases "cell"/-c, but positional_options_description later still maps positional arguments to the name "cell". With Boost.Program_options, positional mappings use the canonical option name, so this change can break the previous behaviour where cell files could be provided as bare positional arguments; they may no longer be associated with the intended option. To keep positional input working while adding the new name, the positional mapping should be updated to the canonical name or the canonical/alias roles for cytosine_report and cell should be swapped.

[Copilot]

The new skip-header-* option names registered here (skip-header-all, skip-header-cytosine_report, etc.) no longer match the keys used by the header-skipping helpers (which still look for skip-header, skip-header-cell, skip-header-reference, and skip-header-intervals). Because Boost.Program_options stores the value under the canonical (first) option name only, variables_map will never contain entries with the alias names used in the helpers, so user-supplied --skip-header* flags will effectively be ignored and header skipping will stay at 0 lines. To preserve backwards compatibility and make the new names work, the helpers should be updated to check the canonical option keys introduced here (or the option names here should revert to the ones expected by getSkipHeaderTemplate).

Suggested change

	("skip-header-all,skip-header",
	po::value<unsigned int>()->implicit_value(1),
	"Header lines to skip in all input files (default: 0). "
	"Synonyms: --skip-header-all, --skip-header.")
	("skip-header-cytosine_report,skip-header-cell",
	po::value<unsigned int>()->implicit_value(1),
	"Header lines to skip in cytosine_report/cell files (default: 0).")
	("skip-header-cytosine_locations,skip-header-reference",
	po::value<unsigned int>()->implicit_value(1),
	"Header lines to skip in cytosine_locations/reference file (default: 0).")
	("skip-header-regions,skip-header-features,skip-header-target,skip-header-intervals",
	("skip-header,skip-header-all",
	po::value<unsigned int>()->implicit_value(1),
	"Header lines to skip in all input files (default: 0). "
	"Synonyms: --skip-header-all, --skip-header.")
	("skip-header-cell,skip-header-cytosine_report",
	po::value<unsigned int>()->implicit_value(1),
	"Header lines to skip in cytosine_report/cell files (default: 0).")
	("skip-header-reference,skip-header-cytosine_locations",
	po::value<unsigned int>()->implicit_value(1),
	"Header lines to skip in cytosine_locations/reference file (default: 0).")
	("skip-header-intervals,skip-header-regions,skip-header-features,skip-header-target",

[Copilot]

The README describes --skip-header-all as having default 1 header line (and similarly implies 1 for the file-specific skip-header options), but the implementation uses implicit_value(1) without a default_value, and getSkipHeaderTemplate returns 0 when these options are absent, so the true default is 0 skipped lines. This discrepancy can confuse users about how their data is parsed; the documentation should be updated to state the correct default or the code should be changed to match the documented default.

Suggested change

	input files (default: 1). Synonyms:
	--skip-header-all, --skip-header.
	--skip-header-cytosine_report [=arg(=1)]
	Number of header lines to skip in
	cytosine_report/cell files (default:
	1).
	--skip-header-cytosine_locations [=arg(=1)]
	Number of header lines to skip in
	cytosine_locations/reference file
	(default: 1).
	--skip-header-regions [=arg(=1)] Number of header lines to skip in
	regions/features/target/intervals file
	(default: 1).
	input files (default: 0; if the option
	is provided without a value, 1 is
	used). Synonyms: --skip-header-all,
	--skip-header.
	--skip-header-cytosine_report [=arg(=1)]
	Number of header lines to skip in
	cytosine_report/cell files (default: 0;
	if the option is provided without a
	value, 1 is used).
	--skip-header-cytosine_locations [=arg(=1)]
	Number of header lines to skip in
	cytosine_locations/reference file
	(default: 0; if the option is provided
	without a value, 1 is used).
	--skip-header-regions [=arg(=1)] Number of header lines to skip in
	regions/features/target/intervals file
	(default: 0; if the option is provided
	without a value, 1 is used).

[Copilot]

In the output section, the --all-meth option is listed twice in succession, which makes the CLI synopsis harder to read and could be mistaken for two separate flags. Please remove the duplicate --all-meth line so that each option appears only once.

Suggested change

--all-meth [=arg(=true)] (=false)

[Copilot]

There are a couple of minor spacing/grammar issues in this block (e.g. "reports.Synonyms" and "globally)Synonyms" without a separating space), which slightly reduce readability of the CLI docs. Adding a space after the period and before Synonyms in both cases will make the text clearer.

Suggested change

	reports.Synonyms: --cytosine_locations,
	--reference, -r.
	Required to reconstruct contiguous CpG
	sequences. Columns:
	chr pos
	-i [ --regions ] arg BED file defining genomic regions where
	entropies will be computed (both per
	feature and globally)Synonyms:
	reports. Synonyms: --cytosine_locations,
	--reference, -r.
	Required to reconstruct contiguous CpG
	sequences. Columns:
	chr pos
	-i [ --regions ] arg BED file defining genomic regions where
	entropies will be computed (both per
	feature and globally) Synonyms:

[imallona]

ok, unmergeable then until the CLI args are updated in a reasonable way. Alternatives: keep the current arg names and just update their description (CLI help); or change the default/canonical arg names everywhere.