`flepimop patch` Abilities And Documentation #423

TimothyWillard · 2024-12-10T20:27:41Z

Describe your changes.

This pull request is in response to #375 (comment).

Does this pull request make any user interface changes? If so please describe.

The user interface changes are...

Those are reflected in updates to the documentation in ...

Tag relevant team members.

Added a unit test to demonstrate the issue described in #375 (comment) with respect to the `seir::parameters` section. Appears to be related to if the parameters are a list or a dict.

TimothyWillard · 2024-12-10T20:33:38Z

Commit b21cfcc demonstrates the seir::parameters issue described in the first bullet point of #375 (comment). It appears that it can append values when it is a dictionary, but does not do so when it is a list.

pearsonca · 2024-12-10T21:35:03Z

Commit b21cfcc demonstrates the seir::parameters issue described in the first bullet point of #375 (comment). It appears that it can append values when it is a dictionary, but does not do so when it is a list.

... I hate confuse.

Okay, the short circuit solution here is to hard error rather than warn on top level key collisions. I'm fine with that for now - thoughts?

Corrected an issue where `parse_config_files` would try to merge top-level keys which comes from `confuse`. Work around is to manually build a dictionary and then convert that to a `Configuration` object.

For configuration of the outputted yaml.

TimothyWillard · 2024-12-11T13:59:57Z

@pearsonca 02678a3 restores the documented behavior.

@saraloo fe1dc2b adds an --indent option for configuring the appearance of the output.

flepimop/gempyor_pkg/src/gempyor/cli.py

flepimop/gempyor_pkg/src/gempyor/shared_cli.py

flepimop/gempyor_pkg/tests/cli/test_flepimop_patch_cli.py

pearsonca · 2024-12-11T16:49:34Z

quick note: i thought we had settings such that -h or --help would work for CLI actions, and also that bare command invocation would trigger the help notes. doesn't seem to be the case -- checking the click docs as to how to have that behavior.

Added an internal utility to custom format the yaml representation of a `confuse.Configuration` object. Creates a custom `yaml.Dumper` class that contains all of the custom representation changes as to not modify the `yaml` pkg defaults.

pearsonca · 2024-12-11T18:33:26Z

flepimop/gempyor_pkg/src/gempyor/cli.py

+def patch(ctx: click.Context = mock_context, **kwargs) -> None:
+    """Merge configuration files
+
+    This command will merge multiple config files together by overriding the top level


We should frame the example in the way we actually expect people to use the tool: flepimop patch config1.yml config2.yml > confignew.yml. Showing the output still useful so maybe do the piping, followed by a cat?

Ah, good point, I've amended the last commit to reflect this.

@pearsonca

Changed the help page to use existing files in the `examples/tutorials` directory rather than construction test configs on the fly per a suggestion by @pearsonca.

TimothyWillard · 2024-12-11T20:07:32Z

This PR:

Upgrades the warning for overriding top level keys to an error so it is not possible now,
Expands the documentation both on the command itself and in the gitbook,
No longer writes a separate seir_modifiers_scenarios or outcome_modifiers_scenarios top level key,
Custom formatting for the patched yaml file to conform with flepiMoP user expectations, and
Unit testing to demonstrate said behavior.

saraloo · 2024-12-11T20:28:51Z

documentation/gitbook/how-to-run/multi-configs.md

 ```

 You may provide an arbitrary number of separate configuration files to combine to create a complete configuration.

 ## Caveats

-At this time, only `simulate` supports multiple configuration files. Also, the patching operation is fairly crude: configuration options override previous ones completely, though with a warning. The files provided from left to right are from lowest priority (i.e. for the first file, only options specified in no other files are used) to highest priority (i.e. for the last file, its options override any other specification).
+At this time, only `simulate` supports multiple configuration files. Also, the patching operation is fairly crude: if multiple configuration files specify the same option, this will yield an error.


Propose just a clarifying statement to make things a bit more explicit.

At this time, only simulate directly supports multiple configuration files, and our current patching capabilities only allow for the addition of new sections as given in our tutorials. This is helpful for building models piece-by-piece from a simple compartmental forward simulation, to including outcome probabilities, and finally, adding modifier sections. If multiple configuration files specify the same higher level configuration chunks (e.g., seir, outcomes), this will yield an error.

saraloo · 2024-12-11T20:30:20Z

documentation/gitbook/how-to-run/multi-configs.md

@@ -32,46 +32,17 @@ you'll get a basic foward simulation of this example model. However, you might a
 flepimop simulate config_sample_2pop.yml config_sample_2pop_outcomes_part.yml
 ```

-if want to see what the combined configuration is, you can use the `patch` command:
+if want to create a combined configuration (e.g. to check what the combined configuration is), you can use the `patch` command:


I think we should suggest this as a way to start - should look at the config before you run it. Maybe for later, should include that simulate should print out the patched config regardless - so it can be tracked what exactly was run.

perhaps change this wording to:

While simulate can run your patched configuration, we also suggest you check your configuration file using the patch command:

@saraloo

Stronger and clearer wording suggested by @saraloo.

TimothyWillard added bug Defects or errors in the code. documentation Relating to ReadMEs / gitbook / vignettes / etc. gempyor Concerns the Python core. high priority High priority. next release Marks a PR as a target to include in the next release. labels Dec 10, 2024

flepimop patch unit test for seir::parameters

b21cfcc

Added a unit test to demonstrate the issue described in #375 (comment) with respect to the `seir::parameters` section. Appears to be related to if the parameters are a list or a dict.

TimothyWillard force-pushed the hotfix/patching-abilities-documentation branch from ef09f6e to b21cfcc Compare December 10, 2024 20:32

TimothyWillard added 2 commits December 11, 2024 08:30

Fix parse_config_files TL key override behavior

02678a3

Corrected an issue where `parse_config_files` would try to merge top-level keys which comes from `confuse`. Work around is to manually build a dictionary and then convert that to a `Configuration` object.

Add --indent option to flepimop patch

fe1dc2b

For configuration of the outputted yaml.

TimothyWillard mentioned this pull request Dec 11, 2024

Release cycle, December 2024 #375

Merged

TimothyWillard requested review from pearsonca and saraloo December 11, 2024 14:07

Add detailed example to flepimop patch --help

ba22f0b

TimothyWillard force-pushed the hotfix/patching-abilities-documentation branch from d397eb1 to ba22f0b Compare December 11, 2024 15:18

pearsonca reviewed Dec 11, 2024

View reviewed changes

TimothyWillard added 2 commits December 11, 2024 11:29

Force default indent of flepimop patch to 4

d3f89d2

Upgrade TL key collision from warning to error

b5142d9

modify approach to handling help

b34792d

TimothyWillard force-pushed the hotfix/patching-abilities-documentation branch from e6ced61 to b34792d Compare December 11, 2024 17:24

Add _dump_formatted_yaml for custom yaml dump

7ae1037

Added an internal utility to custom format the yaml representation of a `confuse.Configuration` object. Creates a custom `yaml.Dumper` class that contains all of the custom representation changes as to not modify the `yaml` pkg defaults.

pearsonca reviewed Dec 11, 2024

View reviewed changes

flepimop patch --help uses example configs

0fe483e

Changed the help page to use existing files in the `examples/tutorials` directory rather than construction test configs on the fly per a suggestion by @pearsonca.

TimothyWillard force-pushed the hotfix/patching-abilities-documentation branch from 8de5973 to 0fe483e Compare December 11, 2024 18:36

pearsonca and others added 3 commits December 11, 2024 13:47

update multi-configs gitbook

719c23b

parse_config_files error includes config name

11d8a2d

Resolve seir/outcome modifier scenarios

ba9b527

saraloo reviewed Dec 11, 2024

View reviewed changes

flepimop patch documenation edits

c88c524

Stronger and clearer wording suggested by @saraloo.

TimothyWillard merged commit c5bde34 into dev Dec 11, 2024
2 checks passed

TimothyWillard deleted the hotfix/patching-abilities-documentation branch December 11, 2024 21:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`flepimop patch` Abilities And Documentation #423

`flepimop patch` Abilities And Documentation #423

TimothyWillard commented Dec 10, 2024

TimothyWillard commented Dec 10, 2024

pearsonca commented Dec 10, 2024

TimothyWillard commented Dec 11, 2024

pearsonca commented Dec 11, 2024

pearsonca Dec 11, 2024

TimothyWillard Dec 11, 2024

TimothyWillard commented Dec 11, 2024

saraloo Dec 11, 2024

saraloo Dec 11, 2024

saraloo Dec 11, 2024

flepimop patch Abilities And Documentation #423

flepimop patch Abilities And Documentation #423

Conversation

TimothyWillard commented Dec 10, 2024

Describe your changes.

Does this pull request make any user interface changes? If so please describe.

Tag relevant team members.

TimothyWillard commented Dec 10, 2024

pearsonca commented Dec 10, 2024

TimothyWillard commented Dec 11, 2024

pearsonca commented Dec 11, 2024

pearsonca Dec 11, 2024

Choose a reason for hiding this comment

TimothyWillard Dec 11, 2024

Choose a reason for hiding this comment

TimothyWillard commented Dec 11, 2024

saraloo Dec 11, 2024

Choose a reason for hiding this comment

saraloo Dec 11, 2024

Choose a reason for hiding this comment

saraloo Dec 11, 2024

Choose a reason for hiding this comment

`flepimop patch` Abilities And Documentation #423

`flepimop patch` Abilities And Documentation #423