First iteration of a CLI based on `pydantic` models that allows to run the `hello_looper` example

simeoncarstens commented 5 months ago

[!NOTE] This description was mostly written by @zz1874, and slightly edited by me.

This PR introduces the initial version of a CLI based on pydantic models, paving the way for a synchronized HTTP API with the looper CLI. The primary goal is to define looper commands and their arguments using pydantic models which are then used for creating a CLI and an HTTP API and later possibly a web frontend application from a common source of truth. The ultimate objective is to replace the existing CLI with one based on pydantic models. This transition aims to enhance clarity, maintainability, and consistency in defining and handling CLI arguments - for example, pydantic-argparse or other, similar libraries (cf. #438) assure that CLI arguments strictly type-checked.

Changes made:

Added command_models module: Introduced a new module called command_models to define pydantic models for looper commands. pydantic models capture both commands and their associated arguments. :warning: some arguments are now specific to the run command instead of being top-level arguments :warning:
Created cli_pydantic.py script: Developed a CLI script named cli_pydantic.py that utilizes pydantic-argparse for parsing command-line arguments. The script leverages pydantic models from the command_models module to represent and validate CLI arguments.
Adjustments in multiple looper files to accommodate the changes: Specifically, the Runner executor has been changed to reflect the changes in argument hierarchy, and in multiple places in cli_looper.py and other files, subsequent changes were necessary to reflect this hierarchy and thus changed location of arguments in argparse namespaces. This "special case treatment" for the run command is not pretty, but if, eventually, the existing argparse CLI is fully replaced by a CLI based on the pydantic models proposed in this PR, will be new default.

Usage and testing:

The command model code is so far only manually tested. We successfully executed the hello_looper example using the new CLI by running

$ cd looper
$ pip install -e .
$ cd path/to/hello_looper-master
$ looper-pydantic-argparse --looper-config .looper.yaml run

The existing pytest test suite passes.

We highly encourage testing with more advanced looper run applications beyond hello_looper, and we are seeking feedback to ensure the new CLI works seamlessly and meets expectations. But as we are no looper users, help on this from regular users would be much appreciated.

Next steps:

Continuation of refining and expanding the pydantic-based CLI to cover a wider range of looper functionality (additional commands such as runp and check and arguments that might not be required for hello_looper) (#448)
Commencing the design phase and building a first prototype for the HTTP API to proof feasibility and synchronization with the pydantic-based CLI (#441)

donaldcampbelljr commented 5 months ago

I attempted to test this on my machine, installing requirements and looper from the tweag/run-hello-world branch. Ran into a module import issue during run: running looper-pydantic-argparse yields:

Traceback (most recent call last):
  File "/home/drc/GITHUB/looper/master/looper/venv/bin/looper-pydantic-argparse", line 5, in <module>
    from looper.cli_pydantic import main
  File "/home/drc/GITHUB/looper/master/looper/venv/lib/python3.10/site-packages/looper/__init__.py", line 14, in <module>
    from .divvy import ComputingConfiguration, select_divvy_config
  File "/home/drc/GITHUB/looper/master/looper/venv/lib/python3.10/site-packages/looper/divvy.py", line 22, in <module>
    from .utils import write_submit_script
  File "/home/drc/GITHUB/looper/master/looper/venv/lib/python3.10/site-packages/looper/utils.py", line 21, in <module>
    from .command_models.commands import SUPPORTED_COMMANDS
ModuleNotFoundError: No module named 'looper.command_models'

khoroshevskyi commented 5 months ago

I attempted to test this on my machine, installing requirements and looper from the tweag/run-hello-world branch. Ran into a module import issue during run: running looper-pydantic-argparse yields: @donaldcampbelljr you should add this module to MANIFEST.in file and it will solve this issue

donaldcampbelljr commented 5 months ago

I attempted to test this on my machine, installing requirements and looper from the tweag/run-hello-world branch. Ran into a module import issue during run: running looper-pydantic-argparse yields: @donaldcampbelljr you should add this module to MANIFEST.in file and it will solve this issue

Thanks, I actually tried that and continued to have issues. I'll try again in a bit and report back.

zz1874 commented 5 months ago

Hello @donaldcampbelljr ! Just wanted to double-check, did you run

cd looper
pip install -e .

by any chance? What os are you working on? cc @simeoncarstens

donaldcampbelljr commented 5 months ago

@zz1874 Oh it works this morning. Odd. Last week I did pip install . (or I thought I did). Well, glad that was an easy solution.

I can confirm the script works on my end. Thanks!

simeoncarstens commented 5 months ago

@donaldcampbelljr did you or anyone else have the chance to test that script for use cases beyond a simple looper-pydantic-argparse --looper-config .looper.yaml run in the hello_looper repository?

donaldcampbelljr commented 5 months ago

I played around with this a bit more this morning. I attempted to use it with a real pipeline example using python 3.8 and was encountering this error:

(pepatac38) drc@databio:~/pepatac_tutorial/tools/pepatac/examples/tutorial$ looper-pydantic-argparse
Traceback (most recent call last):
  File "/home/drc/anaconda3/envs/pepatac38/bin/looper-pydantic-argparse", line 33, in <module>
    sys.exit(load_entry_point('looper', 'console_scripts', 'looper-pydantic-argparse')())
  File "/home/drc/anaconda3/envs/pepatac38/bin/looper-pydantic-argparse", line 25, in importlib_load_entry_point
    return next(matches).load()
  File "/home/drc/anaconda3/envs/pepatac38/lib/python3.8/importlib/metadata.py", line 77, in load
    module = import_module(match.group('module'))
  File "/home/drc/anaconda3/envs/pepatac38/lib/python3.8/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 961, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 843, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/drc/GITHUB/looper/master/looper/looper/__init__.py", line 14, in <module>
    from .divvy import ComputingConfiguration, select_divvy_config
  File "/home/drc/GITHUB/looper/master/looper/looper/divvy.py", line 22, in <module>
    from .utils import write_submit_script
  File "/home/drc/GITHUB/looper/master/looper/looper/utils.py", line 21, in <module>
    from .command_models.commands import SUPPORTED_COMMANDS
  File "/home/drc/GITHUB/looper/master/looper/looper/command_models/commands.py", line 15, in <module>
    class Command:
  File "/home/drc/GITHUB/looper/master/looper/looper/command_models/commands.py", line 26, in Command
    arguments: list[Argument]
TypeError: 'type' object is not subscriptable

A quick google search suggests it is because I'm running 3.8 in this specific example.

Were we still aiming to support 3.8 for this endeavor?

I switched to a python 3.9 environment and attempt to run:

looper-pydantic-argparse --looper-config .looper_tutorial_refgenie_pipestat.yaml run

I get this error ValueError: Looper config file does not exist. Use looper init to create one at .looper.yaml. So I re-name the looper config file to simply be .looper.yaml

Running looper-pydantic-argparse --looper-config .looper.yaml run then yields this:

run=run(ignore_flags=False, time_delay=0, dry_run=False, command_extra='', command_extra_override='', lump=None, lumpn=None, limit=None, skip=None, divvy=None) settings='' pep_config=None output_dir=None config_file=None looper_config='.looper.yaml' sample_pipeline_interfaces=[] project_pipeline_interfaces=[] amend=[] sel_flag='' exc_flag=''
#########################################
config_file /home/drc/pepatac_tutorial/tools/pepatac/examples/tutorial/tutorial_refgenie_project_config.yaml
pep_config /home/drc/pepatac_tutorial/tools/pepatac/examples/tutorial/tutorial_refgenie_project_config.yaml
output_dir /home/drc/pepatac_tutorial//processed/
pipestat {'results_file_path': '${TUTORIAL}/processed/results_pipeline/{record_identifier}/stats.yaml'}
Traceback (most recent call last):
  File "/home/drc/anaconda3/envs/condalooper_p39/bin/looper-pydantic-argparse", line 33, in <module>
    sys.exit(load_entry_point('looper', 'console_scripts', 'looper-pydantic-argparse')())
  File "/home/drc/GITHUB/looper/master/looper/looper/cli_pydantic.py", line 69, in main
    setattr(args, looper_config_key, looper_config_item)
  File "pydantic/main.py", line 357, in pydantic.main.BaseModel.__setattr__
ValueError: "TopLevelParser" object has no field "pipestat"

For reference here is my looper config file:

name: PEPATAC_tutorial
pep_config: tutorial_refgenie_project_config.yaml

output_dir: "${TUTORIAL}/processed/"
pipeline_interfaces:
  sample: ["${TUTORIAL}/tools/pepatac/sample_pipeline_interface.yaml"]
  project: ["${TUTORIAL}/tools/pepatac/project_pipeline_interface.yaml"]

pipestat:
  results_file_path: "${TUTORIAL}/processed/results_pipeline/{record_identifier}/stats.yaml"

So it appears that, at the moment:

we need to be on python 3.9 or higher
we cannot input any path to a looper config file, it must be a .looper.yaml file
unknown items in the config file (in this case, pipestat) will cause an error

simeoncarstens commented 5 months ago

Excellent, thanks @donaldcampbelljr! We will look into these points right away. We were aware about the Python 3.8 incompatibility and will definitely fix this. As for your point 3), I think we might just be missing some of the options in our argument list. We will go through the options defined in cli_looper.py and make sure nothing falls through the cracks. As for 2), we'll debug that, too! Thanks also for sharing your looper config file, that will help us with all that :slightly_smiling_face:

zz1874 commented 5 months ago

Hello @donaldcampbelljr , we appreciate your efforts in experimenting with it! :pray:

Here are the improvements we made:

Rectified typings to ensure compatibility with both Python 3.8 and Python 3.9+.
Addressed the --looper-config option, allowing users to input any path of their choice. (This fix was implemented on a separate branch, and we appreciate you bringing it to our attention!)
Introduced pipestat as a CLI argument, and I've personally tested it on my end to confirm that the bug should be resolved.

Your feedback is incredibly valuable, especially as we lack real-world examples for testing on our end. Please let us know if these changes successfully addressed all the issues you encountered. Thank you!

simeoncarstens commented 5 months ago

@donaldcampbelljr we are in the process of cleaning up this branch a little bit (it contains more than what is strictly necessary to run hello_looper) and transferred some of the commits to a new branch, https://github.com/pepkit/looper/tree/tweag/remaining-run-arguments. So if you want to try again, best pull that latter branch :slightly_smiling_face:

simeoncarstens commented 5 months ago

@donaldcampbelljr see #448, which should make your use case work. @donaldcampbelljr @nsheff are you aware of a way to get a comprehensive list of arguments (both stemming from the CLI and from the config file) that need to be supported? I'd like to avoid incrementally adding arguments to our argument list and having you have to constantly retest your favorite commands.

simeoncarstens commented 5 months ago

In order to avoid proliferation of multiple dependent PRs, we would like to merge this as soon as possible. @donaldcampbelljr could I ask you to review this also on the code level? Especially feedback on the mechanism of building the pydantic models would be appreciated. There are also changes in looper.py, but we still have to add commits that refactor a bit mostly cli_looper.py to make the tests pass. So that part isn't quite ready to review yet.

donaldcampbelljr commented 5 months ago

@donaldcampbelljr see #448, which should make your use case work. @donaldcampbelljr @nsheff are you aware of a way to get a comprehensive list of arguments (both stemming from the CLI and from the config file) that need to be supported? I'd like to avoid incrementally adding arguments to our argument list and having you have to constantly retest your favorite commands.

@simeoncarstens @zz1874 One of the more complicated config files can be found under the documentation that discusses integration with pipestat (for looper > v1.6.0):

https://looper.databio.org/en/latest/pipestat/

e.g.

pep_config: project_config_pipestat.yaml # pephub registry path or local path
output_dir: output
sample_table: annotation_sheet.csv
pipeline_interfaces:
  sample:  ./pipeline_interface1_sample_pipestat.yaml
  project: ./pipeline_interface1_project_pipestat.yaml
pipestat:
  project_name: TEST_PROJECT_NAME
  results_file_path: tmp_pipestat_results.yaml
  flag_file_dir: output/results_pipeline
  database:
    dialect: postgresql
    driver: psycopg2
    name: pipestat-test
    user: postgres
    password: pipestat-password
    host: 127.0.0.1
    port: 5432

simeoncarstens commented 5 months ago

@donaldcampbelljr this is now complete, we believe - the existing test suite passes. We haven't had time to write tests for the (little) new code, unfortunately :slightly_smiling_face: We hope the existing tests cover the refactorings / changes made to code in looper.py / cli_looper.py/ ... I also updated the PR description, and notably added a comment about the unfortunately currently necessary special treatment of the run command because of strict argument hierarchy. Please let us know if you have any questions or remarks!

donaldcampbelljr commented 5 months ago

Pulling tweag/run-hello-world and doing a fresh install with above instructions, I am seeing some testing failures when running pytest tests/.

Could you guys confirm you have all tests passing on your end?

FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit0] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit1] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit2] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit3] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit4] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit5] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit6] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit7] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit8] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit9] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit10] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit11] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit12] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit13] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_limited-arg_and_limit14] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit0] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit1] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit2] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit3] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit4] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit5] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit6] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit7] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit8] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit9] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit10] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit11] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit12] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit13] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_invalid_input__raise_expected_error[desired_samples_range_skipped-arg_and_limit14] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_valid_input__yields_correct_range[2-6-4-desired_samples_range_limited-expected0] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_valid_input__yields_correct_range[3:6-8-desired_samples_range_limited-expected1] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_valid_input__yields_correct_range[2:6-4-desired_samples_range_skipped-expected4] - NameError: name 'defaultdict' is not defined
FAILED tests/test_desired_sample_range.py::test_valid_input__yields_correct_range[3-6-8-desired_samples_range_skipped-expected5] - NameError: name 'defaultdict' is not defined
FAILED tests/smoketests/test_other.py::TestLooperPipestat::test_pipestat_configured[run] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_other.py::TestSelector::test_selecting_flags_works[PIPELINE1-completed] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_other.py::TestSelector::test_excluding_flags_works[PIPELINE1-completed] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_other.py::TestSelector::test_excluding_multi_flags_works[PIPELINE1-completed] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_other.py::TestSelector::test_selecting_multi_flags_works[PIPELINE1-completed] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_other.py::TestSelector::test_selecting_attr_and_flags_works[PIPELINE1-completed] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_other.py::TestSelector::test_excluding_attr_and_flags_works[PIPELINE1-completed] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_other.py::TestSelector::test_excluding_toggle_attr[PIPELINE1-completed] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_other.py::TestSelector::test_including_toggle_attr[PIPELINE1-completed] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::test_cli - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperBothRuns::test_looper_cfg_required[run] - AttributeError: 'Namespace' object has no attribute 'run'
FAILED tests/smoketests/test_run.py::TestLooperBothRuns::test_looper_cfg_required[runp] - Failed: DID NOT RAISE <class 'SystemExit'>
FAILED tests/smoketests/test_run.py::TestLooperBothRuns::test_cmd_extra_cli[arg0-run] - Failed: DID RAISE 'Namespace' object has no attribute 'run'
FAILED tests/smoketests/test_run.py::TestLooperBothRuns::test_cmd_extra_cli[arg1-run] - Failed: DID RAISE 'Namespace' object has no attribute 'run'
FAILED tests/smoketests/test_run.py::TestLooperBothRuns::test_cmd_extra_cli[arg2-run] - Failed: DID RAISE 'Namespace' object has no attribute 'run'
FAILED tests/smoketests/test_run.py::TestLooperBothRuns::test_cmd_extra_cli[arg3-run] - Failed: DID RAISE 'Namespace' object has no attribute 'run'
FAILED tests/smoketests/test_run.py::TestLooperBothRuns::test_unrecognized_args_not_passing[run] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_looper_run_basic - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_looper_multi_pipeline - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_looper_single_pipeline - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_looper_var_templates - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_looper_cli_pipeline - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_looper_no_pipeline - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_looper_pipeline_not_found - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_looper_pipeline_invalid - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_looper_sample_attr_missing - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_looper_toggle - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_cmd_extra_sample[string] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_cmd_extra_sample[ --string] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_cmd_extra_sample[ --sjhsjd 212] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_cmd_extra_sample[7867#$@#$cc@@] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_cmd_extra_override_sample[string] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_cmd_extra_override_sample[ --string] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_cmd_extra_override_sample[ --sjhsjd 212] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunBehavior::test_cmd_extra_override_sample[7867#$@#$cc@@] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunPreSubmissionHooks::test_looper_basic_plugin - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunPreSubmissionHooks::test_looper_other_plugins[looper.write_submission_yaml-submission.yaml] - Failed: DID RAISE 'Namespace' object has no attribute 'run'
FAILED tests/smoketests/test_run.py::TestLooperRunPreSubmissionHooks::test_looper_other_plugins[looper.write_sample_yaml_prj-prj.yaml] - Failed: DID RAISE 'Namespace' object has no attribute 'run'
FAILED tests/smoketests/test_run.py::TestLooperRunPreSubmissionHooks::test_looper_other_plugins[looper.write_sample_yaml_cwl-cwl.yaml] - Failed: DID RAISE 'Namespace' object has no attribute 'run'
FAILED tests/smoketests/test_run.py::TestLooperRunPreSubmissionHooks::test_looper_command_templates_hooks[touch {looper.output_dir}/submission/{sample.sample_name}_test.txt; {%raw%}echo {}{%endraw%}] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunSubmissionScript::test_looper_run_produces_submission_scripts - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunSubmissionScript::test_looper_lumping - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperRunSubmissionScript::test_looper_limiting - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperCompute::test_looper_respects_pkg_selection[run] - Failed: DID RAISE 'Namespace' object has no attribute 'run'
FAILED tests/smoketests/test_run.py::TestLooperCompute::test_looper_uses_cli_compute_options_spec[run] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperCompute::test_cli_yaml_settings_general[run] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperCompute::test_nonexistent_yaml_settings_disregarded[run] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperCompute::test_cli_yaml_settings_passes_settings[run] - Failed: DID RAISE <class 'Exception'>
FAILED tests/smoketests/test_run.py::TestLooperCompute::test_cli_compute_overwrites_yaml_settings_spec[run] - Failed: DID RAISE 'Namespace' object has no attribute 'run'
FAILED tests/smoketests/test_run.py::TestLooperConfig::test_init_config_file[run] - assert 1 == 0
FAILED tests/smoketests/test_run.py::TestLooperConfig::test_correct_execution_of_config - Failed: DID RAISE 'Namespace' object has no attribute 'run'

simeoncarstens commented 5 months ago

@donaldcampbelljr I (un?)fortunately can't confirm this. I tested it both on my usual setup with Python 3.10 and in a Python 3.8 Docker container, both with fresh clones + pulls of this branch :neutral_face: these errors do seem familiar, though, but should be fixed by now. I guess one of us has an outdated or wrong branch :thinking:

donaldcampbelljr commented 5 months ago

It was on my end. Both I and @khoroshevskyi confirmed the tests are passing.

simeoncarstens commented 5 months ago

Thanks, @donaldcampbelljr and @khoroshevskyi! Glad that the tests are passing the the changes look good to you. Nevertheless, I'd like to explicitly point out the following commits:

They add a not-so-nice special treatment for the run command in multiple places of the looper code. As the updated PR description says,

were necessary to reflect this hierarchy and thus changed location of arguments in argparse namespaces. This "special case treatment" for the run command is not pretty, but if, eventually, the existing argparse CLI is fully replaced by a CLI based on the pydantic models proposed in this PR, will be new default.

Until all commands have been migrated to the strict hierarchy and pydantic model definition, a mix of two different namespace structures will coexist. Unfortunately, we don't have the time to adapt the remaining commands, which would simplify this and clean it up somewhat. We'd be happy to discuss in a meeting how your team could best go from there with these changes.

pepkit / looper

First iteration of a CLI based on `pydantic` models that allows to run the `hello_looper` example #440

Changes made:

Usage and testing: