Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hictk: support replaying warnings right before app exit #313

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/hictk/cli/cli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "hictk/tools/cli.hpp"

#include <fmt/format.h>
#include <spdlog/spdlog.h>

#include <CLI/CLI.hpp>
#include <cassert>
Expand All @@ -25,6 +26,13 @@
return Cli::subcommand_to_str(get_subcommand());
}

void Cli::log_warnings() const noexcept {
for (const auto& w : _warnings) {
SPDLOG_WARN(FMT_STRING("{}"), w);
}

Check warning on line 32 in src/hictk/cli/cli.cpp

View check run for this annotation

Codecov / codecov/patch

src/hictk/cli/cli.cpp#L31-L32

Added lines #L31 - L32 were not covered by tests
_warnings.clear();
}

auto Cli::parse_arguments() -> Config {
try {
_cli.name(_exec_name);
Expand Down
13 changes: 4 additions & 9 deletions src/hictk/cli/cli_dump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@
void Cli::validate_dump_subcommand() const {
assert(_cli.get_subcommand("dump")->parsed());

std::vector<std::string> warnings;
std::vector<std::string> errors;
const auto& c = std::get<DumpConfig>(_config);

Expand All @@ -164,26 +163,26 @@
const auto resolution_parsed = !subcmd.get_option("--resolution")->empty();

if ((is_cooler || is_scool) && resolution_parsed) {
warnings.emplace_back("--resolution is ignored when file is in .[s]cool format.");
_warnings.emplace_back("--resolution is ignored when file is in .[s]cool format.");

Check warning on line 166 in src/hictk/cli/cli_dump.cpp

View check run for this annotation

Codecov / codecov/patch

src/hictk/cli/cli_dump.cpp#L166

Added line #L166 was not covered by tests
}

const auto range_parsed = !subcmd.get_option("--range")->empty();
if (range_parsed && c.table != "chroms" && c.table != "bins" && c.table != "pixels" &&
c.table != "weights") {
warnings.emplace_back(
_warnings.emplace_back(

Check warning on line 172 in src/hictk/cli/cli_dump.cpp

View check run for this annotation

Codecov / codecov/patch

src/hictk/cli/cli_dump.cpp#L172

Added line #L172 was not covered by tests
"--range and --range2 are ignored when --table is not bins, chroms, pixels, or weights");
}

const auto query_file_parsed = !subcmd.get_option("--query-file")->empty();
if (query_file_parsed && c.table != "bins" && c.table != "pixels") {
warnings.emplace_back("--query-file is ignored when --table is not bins or pixels");
_warnings.emplace_back("--query-file is ignored when --table is not bins or pixels");

Check warning on line 178 in src/hictk/cli/cli_dump.cpp

View check run for this annotation

Codecov / codecov/patch

src/hictk/cli/cli_dump.cpp#L178

Added line #L178 was not covered by tests
}

const auto matrix_type_parsed = !subcmd.get_option("--matrix-type")->empty();
const auto matrix_unit_parsed = !subcmd.get_option("--matrix-unit")->empty();

if (!is_hic && (matrix_type_parsed || matrix_unit_parsed)) {
warnings.emplace_back(
_warnings.emplace_back(

Check warning on line 185 in src/hictk/cli/cli_dump.cpp

View check run for this annotation

Codecov / codecov/patch

src/hictk/cli/cli_dump.cpp#L185

Added line #L185 was not covered by tests
"--matrix-type and --matrix-unit are ignored when input file is not in .hic format.");
}

Expand All @@ -205,10 +204,6 @@
errors.emplace_back("--balance requires --table=pixels.");
}

for (const auto& w : warnings) {
SPDLOG_WARN(FMT_STRING("{}"), w);
}

if (!errors.empty()) {
throw std::runtime_error(
fmt::format(FMT_STRING("the following error(s) where encountered while validating CLI "
Expand Down
15 changes: 5 additions & 10 deletions src/hictk/cli/cli_fix_mcool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@
void Cli::validate_fix_mcool_subcommand() const {
const auto& c = std::get<FixMcoolConfig>(_config);
std::vector<std::string> errors;
std::vector<std::string> warnings{};

if (!c.force && std::filesystem::exists(c.path_to_output)) {
errors.emplace_back(fmt::format(
Expand All @@ -113,20 +112,20 @@
if (c.skip_balancing) {
const auto* sc = _cli.get_subcommand("fix-mcool");
if (!sc->get_option("--tmpdir")->empty()) {
warnings.emplace_back("option --tmpdir is ignored when --skip-balancing is provided.");
_warnings.emplace_back("option --tmpdir is ignored when --skip-balancing is provided.");

Check warning on line 115 in src/hictk/cli/cli_fix_mcool.cpp

View check run for this annotation

Codecov / codecov/patch

src/hictk/cli/cli_fix_mcool.cpp#L115

Added line #L115 was not covered by tests
}
if (!sc->get_option("--in-memory")->empty()) {
warnings.emplace_back("option --in-memory is ignored when --skip-balancing is provided.");
_warnings.emplace_back("option --in-memory is ignored when --skip-balancing is provided.");

Check warning on line 118 in src/hictk/cli/cli_fix_mcool.cpp

View check run for this annotation

Codecov / codecov/patch

src/hictk/cli/cli_fix_mcool.cpp#L118

Added line #L118 was not covered by tests
}
if (!sc->get_option("--compression-lvl")->empty()) {
warnings.emplace_back(
_warnings.emplace_back(

Check warning on line 121 in src/hictk/cli/cli_fix_mcool.cpp

View check run for this annotation

Codecov / codecov/patch

src/hictk/cli/cli_fix_mcool.cpp#L121

Added line #L121 was not covered by tests
"option --compression-lvl is ignored when --skip-balancing is provided.");
}
if (!sc->get_option("--chunk-size")->empty()) {
warnings.emplace_back("option --chunk-size is ignored when --skip-balancing is provided.");
_warnings.emplace_back("option --chunk-size is ignored when --skip-balancing is provided.");

Check warning on line 125 in src/hictk/cli/cli_fix_mcool.cpp

View check run for this annotation

Codecov / codecov/patch

src/hictk/cli/cli_fix_mcool.cpp#L125

Added line #L125 was not covered by tests
}
if (!sc->get_option("--threads")->empty()) {
warnings.emplace_back("option --threads is ignored when --skip-balancing is provided.");
_warnings.emplace_back("option --threads is ignored when --skip-balancing is provided.");

Check warning on line 128 in src/hictk/cli/cli_fix_mcool.cpp

View check run for this annotation

Codecov / codecov/patch

src/hictk/cli/cli_fix_mcool.cpp#L128

Added line #L128 was not covered by tests
}
}

Expand All @@ -137,10 +136,6 @@
FMT_STRING("fixing .mcool with storage-mode=\"{}\" is not supported"), *storage_mode));
}

for (const auto& w : warnings) {
SPDLOG_WARN(FMT_STRING("{}"), w);
}

if (!errors.empty()) {
throw std::runtime_error(fmt::format(
FMT_STRING(
Expand Down
7 changes: 1 addition & 6 deletions src/hictk/cli/cli_load.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@
void Cli::validate_load_subcommand() const {
assert(_cli.get_subcommand("load")->parsed());

std::vector<std::string> warnings;
std::vector<std::string> errors;
const auto& c = std::get<LoadConfig>(_config);
const auto& sc = *_cli.get_subcommand("load");
Expand Down Expand Up @@ -230,14 +229,10 @@
}

if (c.format == "4dn" && c.format == "validpairs" && c.assume_sorted) {
warnings.emplace_back(
_warnings.emplace_back(

Check warning on line 232 in src/hictk/cli/cli_load.cpp

View check run for this annotation

Codecov / codecov/patch

src/hictk/cli/cli_load.cpp#L232

Added line #L232 was not covered by tests
"--assume-sorted has no effect when ingesting interactions in 4dn or validpairs format.");
}

for (const auto& w : warnings) {
SPDLOG_WARN(FMT_STRING("{}"), w);
}

if (!errors.empty()) {
throw std::runtime_error(
fmt::format(FMT_STRING("the following error(s) where encountered while validating CLI "
Expand Down
9 changes: 2 additions & 7 deletions src/hictk/cli/cli_zoomify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@
void Cli::validate_zoomify_subcommand() const {
assert(_cli.get_subcommand("zoomify")->parsed());

std::vector<std::string> warnings;
std::vector<std::string> errors;
const auto& c = std::get<ZoomifyConfig>(_config);

Expand Down Expand Up @@ -206,7 +205,7 @@

if (base_resolution == 0) { // Variable bin size
errors.clear();
warnings.clear();
_warnings.clear();
errors.emplace_back("Zoomifying files with variable bin size is currently not supported.");
} else {
if (const auto dupl = detect_duplicate_resolutions(c.resolutions); !dupl.empty()) {
Expand All @@ -226,16 +225,12 @@
const auto nice_or_pow2_steps_parsed =
!sc->get_option("--nice-steps")->empty() || !sc->get_option("--pow2-steps")->empty();
if (!c.resolutions.empty() && nice_or_pow2_steps_parsed) {
warnings.emplace_back(
_warnings.emplace_back(

Check warning on line 228 in src/hictk/cli/cli_zoomify.cpp

View check run for this annotation

Codecov / codecov/patch

src/hictk/cli/cli_zoomify.cpp#L228

Added line #L228 was not covered by tests
"--nice-steps and --pow2-steps are ignored when resolutions are explicitly set with "
"--resolutions.");
}
}

for (const auto& w : warnings) {
SPDLOG_WARN(FMT_STRING("{}"), w);
}

if (!errors.empty()) {
throw std::runtime_error(
fmt::format(FMT_STRING("the following error(s) where encountered while validating CLI "
Expand Down
2 changes: 2 additions & 0 deletions src/hictk/include/hictk/tools/cli.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ class Cli {
[[nodiscard]] int exit(const CLI::ParseError& e) const;
[[nodiscard]] int exit() const noexcept;
[[nodiscard]] static std::string_view subcommand_to_str(subcommand s) noexcept;
void log_warnings() const noexcept;

private:
int _argc;
Expand All @@ -229,6 +230,7 @@ class Cli {
Config _config{};
CLI::App _cli{};
subcommand _subcommand{subcommand::help};
mutable std::vector<std::string> _warnings{};

void make_balance_subcommand();
void make_ice_balance_subcommand(CLI::App& app);
Expand Down
Loading
Loading