Skip to content
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
39 changes: 13 additions & 26 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::graph::save_commodity_graphs_for_model;
use crate::input::{load_commodity_graphs, load_model};
use crate::log;
use crate::output::{copy_input_files, create_output_directory, get_graphs_dir, get_output_dir};
use crate::settings::Settings;
use crate::settings::{Settings, SettingsOverrides};
use ::log::{info, warn};
use anyhow::{Context, Result};
use clap::{Args, CommandFactory, Parser, Subcommand};
Expand Down Expand Up @@ -41,16 +41,9 @@ pub struct RunOpts {
/// Directory for output files
#[arg(short, long)]
pub output_dir: Option<PathBuf>,
/// Whether to overwrite the output directory if it already exists
#[arg(long)]
pub overwrite: bool,
/// Whether to write additional information to CSV files
#[arg(long)]
pub debug_model: bool,

/// Whether to skip copying input files to the output folder
#[arg(long)]
pub no_copy_input_files: bool,
/// Settings that can be overridden on the command line
#[command(flatten)]
pub overrides: SettingsOverrides,
}
Comment on lines +44 to 47

/// Options for the `graph` command
Expand All @@ -60,8 +53,8 @@ pub struct GraphOpts {
#[arg(short, long)]
pub output_dir: Option<PathBuf>,
/// Whether to overwrite the output directory if it already exists
#[arg(long)]
pub overwrite: bool,
#[arg(long, value_name = "BOOL", num_args = 0..=1, require_equals = true, default_missing_value = "true")]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's all this about?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we still have overwrite here and not in RunOpts?

pub overwrite: Option<bool>,
}

/// The available commands.
Expand Down Expand Up @@ -139,16 +132,8 @@ pub fn run_cli() -> Result<()> {
pub fn handle_run_command(model_path: &Path, opts: &RunOpts) -> Result<()> {
let mut settings = Settings::load_or_default().context("Failed to load settings.")?;

// These settings can be overridden by command-line arguments
if opts.debug_model {
settings.debug_model = true;
}
if opts.overwrite {
settings.overwrite = true;
}
if opts.no_copy_input_files {
settings.copy_input_files = false;
}
// Command-line arguments take precedence over the settings file
settings.apply_overrides(&opts.overrides);

Comment on lines +135 to 137
// Get path to output folder
let pathbuf: PathBuf;
Expand Down Expand Up @@ -220,9 +205,11 @@ pub fn handle_validate_command(model_path: &Path) -> Result<()> {
pub fn handle_save_graphs_command(model_path: &Path, opts: &GraphOpts) -> Result<()> {
let mut settings = Settings::load_or_default().context("Failed to load settings.")?;

if opts.overwrite {
settings.overwrite = true;
}
// Command-line arguments take precedence over the settings file
settings.apply_overrides(&SettingsOverrides {
overwrite: opts.overwrite,
..Default::default()
});

// Get path to output folder
let pathbuf: PathBuf;
Expand Down
158 changes: 131 additions & 27 deletions src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::get_muse2_config_dir;
use crate::input::read_toml;
use crate::log::DEFAULT_LOG_LEVEL;
use anyhow::Result;
use clap::Args;
use documented::DocumentedFields;
use serde::{Deserialize, Serialize};
use std::env;
Expand Down Expand Up @@ -35,38 +36,117 @@ pub fn get_settings_file_path() -> PathBuf {
path
}

/// Program settings from config file
/// Defines the program [`Settings`] together with their defaults and which of them can be
/// overridden from the command line.
///
/// NOTE: If you add or change a field in this struct, you must also update the schema in
/// `schemas/settings.yaml`.
#[derive(Debug, DocumentedFields, Serialize, Deserialize, PartialEq)]
#[serde(default)]
pub struct Settings {
/// Each setting is declared exactly once: its doc comment, type and default value. Appending
/// `, cli` to a setting generates a matching `Option<bool>` field on [`SettingsOverrides`]
/// (reusing the doc comment as the flag's help text) and a merge step in
/// [`Settings::apply_overrides`]. Settings without `cli` can only be set via `settings.toml`.
macro_rules! settings {
(
$(
$(#[doc = $doc:literal])+
$field:ident: $ty:ty = $default:expr $(, $cli:ident)? ;
)+
) => {
/// Program settings, resolved from the defaults, the settings file and CLI overrides.
///
/// NOTE: If you add or change a field here, you must also update the schema in
/// `schemas/settings.yaml`.
#[derive(Debug, DocumentedFields, Serialize, Deserialize, PartialEq)]
#[serde(default)]
pub struct Settings {
$(
$(#[doc = $doc])+
pub $field: $ty,
)+
}

impl Default for Settings {
fn default() -> Self {
Self {
$( $field: $default, )+
}
}
}

settings!(@overrides { } { }
$( [ $field { $(#[doc = $doc])+ } $($cli)? ] )+
);
};

// Overridable setting: add an `Option<bool>` flag and a merge step.
(@overrides
{ $($fields:tt)* }
{ $($applies:tt)* }
[ $field:ident { $(#[doc = $doc:literal])+ } cli ]
$($rest:tt)*
) => {
settings!(@overrides
{
$($fields)*
$(#[doc = $doc])+
#[arg(long, value_name = "BOOL", num_args = 0..=1, require_equals = true, default_missing_value = "true")]
pub $field: Option<bool>,
}
{ $($applies)* ($field) }
$($rest)*
);
};

// File-only setting: skip it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this about?

(@overrides
{ $($fields:tt)* }
{ $($applies:tt)* }
[ $field:ident { $(#[doc = $doc:literal])+ } ]
$($rest:tt)*
) => {
settings!(@overrides { $($fields)* } { $($applies)* } $($rest)*);
};

// No settings left: emit the overrides struct and the merge method.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about settings that are neither bool or file paths?

(@overrides { $($fields:tt)* } { $( ($afield:ident) )* }) => {
/// Settings that can be overridden via command-line arguments.
///
/// A field is `Some` only when the corresponding flag was passed; otherwise the value from
/// the settings file (or default) is kept.
#[derive(Debug, Default, Args)]
pub struct SettingsOverrides {
$($fields)*
}

impl Settings {
/// Apply command-line `overrides` on top of these settings.
pub fn apply_overrides(&mut self, overrides: &SettingsOverrides) {
$(
if let Some(value) = overrides.$afield {
self.$afield = value;
}
)*
}
}
};
}

settings! {
/// The default program log level
pub log_level: String,
/// Whether to overwrite output files by default
pub overwrite: bool,
/// Whether to write additional information to CSV files
pub debug_model: bool,
log_level: String = DEFAULT_LOG_LEVEL.to_string();

/// Results root path to save MUSE2 results. Defaults to `muse2_results`.
pub results_root: PathBuf,
results_root: PathBuf = PathBuf::from("muse2_results");

/// Results root path to save MUSE2 graph outputs. Defaults to `muse2_graphs`.
pub graph_results_root: PathBuf,
/// Whether to copy input files to the output folder.
pub copy_input_files: bool,
}
graph_results_root: PathBuf = PathBuf::from("muse2_graphs");

impl Default for Settings {
fn default() -> Self {
Self {
log_level: DEFAULT_LOG_LEVEL.to_string(),
overwrite: false,
debug_model: false,
results_root: PathBuf::from("muse2_results"),
graph_results_root: PathBuf::from("muse2_graphs"),
copy_input_files: true,
}
}
/// Whether to overwrite output files
overwrite: bool = false, cli;

/// Whether to write additional information to CSV files
debug_model: bool = false, cli;

/// Whether to copy input files to the output folder
copy_input_files: bool = true, cli;
}

impl Settings {
Expand Down Expand Up @@ -129,6 +209,7 @@ impl Settings {
#[cfg(test)]
mod tests {
use super::*;
use rstest::rstest;
use std::fs::File;
use std::io::Write;
use tempfile::tempdir;
Expand Down Expand Up @@ -166,4 +247,27 @@ mod tests {
fn default_file_contents() {
assert!(!Settings::default_file_contents().is_empty());
}

/// A CLI override should always take precedence over the settings-file value, including the
/// case where the user explicitly opts out of an option that is enabled in the file.
#[rstest]
#[case(false, None, false)] // no flag passed: keep the file value
#[case(true, None, true)] // no flag passed: keep the file value
#[case(false, Some(true), true)] // --overwrite (=true): opt in
#[case(true, Some(false), false)] // --overwrite=false: opt out
fn apply_overrides_takes_precedence(
#[case] file_value: bool,
#[case] cli_value: Option<bool>,
#[case] expected: bool,
) {
let mut settings = Settings {
overwrite: file_value,
..Settings::default()
};
settings.apply_overrides(&SettingsOverrides {
overwrite: cli_value,
..Default::default()
});
assert_eq!(settings.overwrite, expected);
}
}
4 changes: 2 additions & 2 deletions tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,15 @@ fn check_example_info_command() {
}

#[test]
fn check_no_copy_input_files_flag() {
fn check_copy_input_files_flag() {
let tempdir = tempdir().unwrap();
let output_dir = tempdir.path().join("results");
assert_muse2_runs(&[
"run",
MODEL_DIR,
"--output-dir",
&output_dir.to_string_lossy(),
"--no-copy-input-files",
"--copy-input-files=false",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an improvement

]);

// Check that input files were not copied to the output directory
Expand Down
2 changes: 1 addition & 1 deletion tests/regenerate_test_data.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ run_example() {

if ! env MUSE2_LOG_LEVEL=error MUSE2_USE_DEFAULT_SETTINGS=1 \
cargo -q run example run -o "data/$example" "$example" \
--overwrite --no-copy-input-files $@; then
--overwrite --copy-input-files=false $@; then
echo ERROR: Failed to run example "$example" > /dev/stderr
failed=1
fi
Expand Down
Loading