From 74427cb1ada6c92e91869df7f725d1601896bdf9 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Tue, 22 Aug 2023 17:44:44 +1000 Subject: [PATCH 01/27] Added __print_usage_pydra__ special options for both Python and C++ commands, for automatic generation of Pydra task wrappers for all MRtrix3 commands. --- .flake8 | 10 + .github/workflows/package-pydra.yml | 243 +++ .gitignore | 28 +- cpp/cmd/amp2response.cpp | 2 +- cpp/core/app.cpp | 1348 +++++++++++------ python/mrtrix3/app.py | 237 +++ .../commands/population_template/usage.py | 13 +- 7 files changed, 1416 insertions(+), 465 deletions(-) create mode 100644 .flake8 create mode 100644 .github/workflows/package-pydra.yml diff --git a/.flake8 b/.flake8 new file mode 100644 index 0000000000..6a8f563a44 --- /dev/null +++ b/.flake8 @@ -0,0 +1,10 @@ + +[flake8] +doctests = True +exclude = + **/__init__.py +max-line-length = 88 +select = C,E,F,W,B,B950 +extend-ignore = E203,E501,E129,E111,E303,E211,E201,E114,E261,E262,E226,E302,E121 +per-file-ignores = + setup.py:F401 diff --git a/.github/workflows/package-pydra.yml b/.github/workflows/package-pydra.yml new file mode 100644 index 0000000000..9018bdfe86 --- /dev/null +++ b/.github/workflows/package-pydra.yml @@ -0,0 +1,243 @@ +#This workflow will install Python dependencies, run tests and lint with a variety of Python versions +# For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions + +# For deployment, it will be necessary to create a PyPI API token and store it as a secret +# https://docs.github.com/en/actions/reference/encrypted-secrets + +name: Pydra + +on: + push: + branches: [ master ] + tags: [ '*' ] + pull_request: + branches: + - master + - dev + +jobs: + + generate-pydra: + + runs-on: ubuntu-latest + + env: + CFLAGS: -Werror + QT_SELECT: qt6 + SCCACHE_GHA_ENABLED: "true" + SCCACHE_CACHE_SIZE: "2G" + + steps: + - uses: actions/checkout@v1 + with: + submodules: true + + - name: Determine MRtrix3 version + run: echo "MRTRIX3_VERSION=$(git describe --tags --abbrev=0)" >> $GITHUB_ENV + + - name: install dependencies + run: | + sudo apt-get update + sudo apt-get install clang qt6-base-dev libglvnd-dev libeigen3-dev zlib1g-dev libfftw3-dev ninja-build + + - name: Run sccache-cache + uses: mozilla-actions/sccache-action@v0.0.3 + + - name: Get CMake + uses: lukka/get-cmake@latest + with: + cmakeVersion: '3.16.3' + + - name: Print CMake version + run: cmake --version + + - name: configure + run: > + cmake + -B build + -G Ninja + -D CMAKE_BUILD_TYPE=Release + -D MRTRIX_BUILD_TESTS=ON + -D MRTRIX_STL_DEBUGGING=ON + -D MRTRIX_WARNINGS_AS_ERRORS=ON + -D CMAKE_C_COMPILER=clang + -D CMAKE_CXX_COMPILER=clang++ + -D CMAKE_INSTALL_PREFIX=$(pwd)/install + + - name: Build Mrtrix + run: cmake --build build + + - name: Install Mrtrix + run: cmake --install build + + - name: Set PATH Variable + run: echo "PATH=$PATH:$(pwd)/install/bin" >> $GITHUB_ENV + + - name: Set LD_LIBRARY_PATH Variable + run: echo "LD_LIBRARY_PATH=$(pwd)/install/lib" >> $GITHUB_ENV + + - name: Set up Python + uses: actions/setup-python@v2 + + - name: Install Python build dependencies + run: | + python -m pip install --upgrade pip + + - name: Install pydra-auto-gen requirements + run: | + pip install -e interfaces/pydra/fileformats-medimage-mrtrix3 + pip install -e interfaces/pydra/fileformats-medimage-mrtrix3-extras + pip install -e interfaces/pydra/src + pip install -r interfaces/pydra/requirements.txt + + - name: Generate task specifications + run: > + ./interfaces/pydra/generate.py + $(pwd)/install/bin + $(pwd)/interfaces/pydra/src + $MRTRIX3_VERSION + --log-errors + --latest + + - name: Upload auto-gen pydra + uses: actions/upload-artifact@v2 + with: + name: AutoGen + path: interfaces/pydra/src/pydra/tasks/mrtrix3/v3_0 + + devcheck: + needs: [generate-pydra] + runs-on: ubuntu-latest + strategy: + matrix: + python-version: [3.8, 3.11] # Check oldest and newest versions + pip-flags: ['', '--editable'] + pydra: + - 'pydra' + - '--editable git+https://github.com/nipype/pydra.git#egg=pydra' + + steps: + - uses: actions/checkout@v2 + + - name: Download auto-gen pydra + uses: actions/download-artifact@v2 + with: + name: AutoGen + path: interfaces/pydra/src/pydra/tasks/mrtrix3/v3_0 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + + - name: Install build dependencies + run: | + python -m pip install --upgrade pip + + - name: Install Pydra + run: | + pip install ${{ matrix.pydra }} + python -c "import pydra as m; print(f'{m.__name__} {m.__version__} @ {m.__file__}')" + + - name: Install task package + run: | + pip install ${{ matrix.pip-flags }} "interfaces/pydra/fileformats-medimage-mrtrix3" + pip install ${{ matrix.pip-flags }} "interfaces/pydra/src[dev]" + python -c "import pydra.tasks.mrtrix3 as m; print(f'{m.__name__} {m.__version__} @ {m.__file__}')" + python -c "import pydra.tasks.mrtrix3.v3_0" + python -c "import pydra as m; print(f'{m.__name__} {m.__version__} @ {m.__file__}')" + + # test: + # needs: [generate] + # runs-on: ubuntu-latest + # strategy: + # matrix: + # python-version: [3.8, 3.11] + # defaults: + # run: + # shell: bash -l {0} + # steps: + # - uses: actions/checkout@v2 + # - name: Install Minconda + # uses: conda-incubator/setup-miniconda@v2 + # with: + # auto-activate-base: true + # activate-environment: "" + # - name: Install MRtrix via Conda + # run: | + # conda install -c mrtrix3 mrtrix3 + # mrconvert --version + # - name: Set up Python ${{ matrix.python-version }} + # uses: actions/setup-python@v2 + # with: + # python-version: ${{ matrix.python-version }} + # - name: Install build dependencies + # run: | + # python -m pip install --upgrade pip + # - name: Install task package + # run: | + # pip install interfaces/pydra/fileformats-medimage-mrtrix interfaces/pydra/fileformats-medimage-mrtrix-extras 'interfaces/pydra/src[test]' + # python -c "import pydra.tasks.mrtrix3 as m; print(f'{m.__name__} {m.__version__} @ {m.__file__}')" + # python -c "import pydra as m; print(f'{m.__name__} {m.__version__} @ {m.__file__}')" + # - name: Test with pytest + # run: | + # pytest -sv --doctest-modules interfaces/src/pydra/tasks/mrtrix3 \ + # --cov pydra.tasks.mrtrix3 --cov-report xml + # - uses: codecov/codecov-action@v1 + # if: ${{ always() }} + + + deploy: + needs: [devcheck] # , test] + runs-on: ubuntu-latest + strategy: + matrix: + python-version: [3.11] + steps: + - uses: actions/checkout@v2 + + - name: Download auto-gen pydra + uses: actions/download-artifact@v2 + with: + name: AutoGen + path: interfaces/pydra/src/pydra/tasks/mrtrix3/v3_0 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + + - name: Install build tools + run: python -m pip install --upgrade pip twine build + + - name: Build source and wheel distributions + working-directory: ./interfaces/pydra/src + run: python -m build + + - name: Check distributions + run: twine check interfaces/pydra/src/dist/* + + - name: Upload sdist + uses: actions/upload-artifact@v2 + with: + name: SDist + path: interfaces/pydra/src/dist/*.tar.gz + + # Deploy on tags if PYPI_API_TOKEN is defined in the repository secrets. + # Secrets are not accessible in the if: condition [0], so set an output variable [1] + # [0] https://github.community/t/16928 + # [1] https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-output-parameter + - name: Check for PyPI token on tag + id: deployable + if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags') + env: + PYPI_API_TOKEN: "${{ secrets.PYPI_API_TOKEN }}" + run: if [ -n "$PYPI_API_TOKEN" ]; then echo ::set-output name=DEPLOY::true; fi + + - name: Upload to PyPI + if: steps.deployable.outputs.DEPLOY + uses: pypa/gh-action-pypi-publish@release/v1 + with: + user: __token__ + password: ${{ secrets.PYPI_API_TOKEN }} + packages-dir: interfaces/pydra/src/dist/ diff --git a/.gitignore b/.gitignore index 38528667db..9f38d7bd50 100644 --- a/.gitignore +++ b/.gitignore @@ -11,12 +11,22 @@ *.img *.hdr *.rste -/python/lib/mrtrix3/_version.py +*.venv +config +build.* +/configure.log +/build.log +/core/version.cpp +/src/exec_version.cpp +/src/project_version.h +/lib/mrtrix3/_version.py +/test/src/project_version.h /scripts/mrtrix_bash_completion /dev/ /compiled_docs/ /.vscode/ .cproject +.DS_store .idea .project .settings @@ -35,3 +45,19 @@ build/ CMakeLists.txt.user testing/data/tmp* testing/data/*-tmp-* +*.venv +/interfaces/pydra/src/pydra/tasks/mrtrix3/* +_version.py +version.cpp +exec_version.cpp +__pycache__ +.DS_Store + +# Pydra related ignores + +_version.py +/interfaces/pydra/src/pydra/tasks/mrtrix3/v3_0 +/interfaces/pydra/**/dist/** +.pytest_cache +/build* +/install diff --git a/cpp/cmd/amp2response.cpp b/cpp/cmd/amp2response.cpp index dc7e0c2930..ea8f3cdbb6 100644 --- a/cpp/cmd/amp2response.cpp +++ b/cpp/cmd/amp2response.cpp @@ -57,7 +57,7 @@ void usage() { ARGUMENTS + Argument ("amps", "the amplitudes image").type_image_in() + Argument ("mask", "the mask containing the voxels from which to estimate the response function").type_image_in() - + Argument ("directions", "a 4D image containing the estimated fibre directions").type_image_in() + + Argument ("directions_image", "a 4D image containing the estimated fibre directions").type_image_in() + Argument ("response", "the output zonal spherical harmonic coefficients").type_file_out(); OPTIONS diff --git a/cpp/core/app.cpp b/cpp/core/app.cpp index 18a3b73f90..12e78e9916 100644 --- a/cpp/core/app.cpp +++ b/cpp/core/app.cpp @@ -847,431 +847,911 @@ std::string restructured_text_usage() { return s; } -const Option *match_option(std::string_view arg) { - auto no_dash_arg = without_leading_dash(arg); - if (arg.size() == no_dash_arg.size() || no_dash_arg.empty() || isdigit(no_dash_arg.front()) != 0 || - no_dash_arg.front() == '.') { - return nullptr; +std::string pydra_usage() { + + std::string CMD_PREFIXES[] = {"Fivett", "Afd", "Amp", "Connectome", "Dcm", "Dir", "Dwi", "Fixel", + "Fod", "Label", "Mask", "Mesh", "Mr", "Mt", "Peaks", "Sh", + "Tck", "Tensor", "Transform", "Tsf", "Voxel", "Vector", "Warp"}; + + auto convert_to_pascal_case = [&](const std::string &input) { + std::string result; + bool capitalizeNext = true; + for (char c : input) { + if (!result.size() && c == '5') { + result += "Five"; // handle 5tt prefixes so they don't create invalid Python identifiers + capitalizeNext = false; + } else if (std::isalpha(c) && capitalizeNext) { + result += std::toupper(c); + capitalizeNext = false; + } else { + result += c; + if (c == '2') + capitalizeNext = true; + for (const std::string &prefix : CMD_PREFIXES) { + if (result == prefix) { + capitalizeNext = true; + break; + } + } + } + } + return result; + }; + + std::string name_string = convert_to_pascal_case(NAME); + // Check whether name starts with 5tt and escape the name if so + + if (name_string.length() > 3) { + std::string prefix = name_string.substr(0, 3); + if (!prefix.compare("5tt")) { + name_string = "Fivett" + name_string.substr(3, name_string.length()); + } } - std::vector candidates; - std::string root(no_dash_arg); + std::string base_indent(" "); + std::string indent = base_indent + " "; + + std::string PYTHON_KEYWORDS[] = { + "and", "as", "assert", "break", "class", "continue", "def", "del", "elif", + "else", "except", "False", "finally", "for", "from", "global", "if", "import", + "in", "is", "lambda", "None", "nonlocal", "not", "or", "pass", "raise", + "return", "True", "try", "while", "with", "yield", "container", "image", "container_xargs"}; + + // Add import lines + std::string s = + std::string("# Auto-generated from MRtrix C++ command with '__print_usage_pydra__' secret option\n\n"); + s += "import typing as ty \n"; + s += "from pathlib import Path # noqa: F401\n"; + s += "from fileformats.generic import File, Directory # noqa: F401\n"; + s += "from fileformats.medimage_mrtrix3 import ImageIn, ImageOut, Tracks # noqa: F401\n"; + s += "from pydra.compose import shell\n"; + s += "from pydra.utils.typing import MultiInputObj\n"; + + auto escape_id = [&](const std::string &id) { + std::string escaped = id; + // Replace any spaces and periods with underscores + std::replace(escaped.begin(), escaped.end(), ' ', '_'); + std::replace(escaped.begin(), escaped.end(), '.', '_'); + // Append any Python keywords with an underscore + bool is_keyword = std::any_of(std::begin(PYTHON_KEYWORDS), + std::end(PYTHON_KEYWORDS), + [&id](const std::string &kword) { return kword == id; }); + if (is_keyword) + escaped += "_"; + + return escaped; + }; - for (size_t i = 0; i < OPTIONS.size(); ++i) - get_matches(candidates, OPTIONS[i], root); - get_matches(candidates, __standard_options, root); + auto format_type = [&](const ArgType &type, bool optional = false) { + switch (type) { + case Undefined: + return "ty.Any"; + case Text: + return "str"; + case Boolean: + return "bool"; + case Integer: + return "int"; + case Float: + return "float"; + case ArgFileIn: + return "File"; + case ArgFileOut: + return "File"; + case ArgDirectoryIn: + return "Directory"; + case ArgDirectoryOut: + return "Directory"; + case Choice: + return "str"; + case ImageIn: + return "ImageIn"; + case ImageOut: + return "ImageOut"; + case IntSeq: + return "list[int]"; + case FloatSeq: + return "list[float]"; + case TracksIn: + return "Tracks"; + case TracksOut: + return "Tracks"; + case Various: + return "ty.Any"; + default: + assert(0); + } + return "not-reached"; + }; - // no matches - if (candidates.empty()) - throw Exception(std::string("unknown option \"-") + root + "\""); + auto format_option_type = [&](const Option &opt, bool for_output = false) { + std::string f; + bool is_multi = (opt.flags & AllowMultiple) && (!opt.size() || opt[0].type != ArgFileOut); + if (is_multi) { + f += "MultiInputObj["; + } + if (!opt.size()) { + f += "bool"; + } else if (opt.size() == 1) { + f += format_type(opt[0].type, true); + } else { + f += "tuple["; + for (size_t a = 0; a < opt.size(); ++a) { + f += format_type(opt[0].type, true); + if (a != opt.size() - 1) { + f += ", "; + } + } + f += "]"; + } + if (is_multi) { + f += "]"; + } + return f; + }; - // return match if unique: - if (candidates.size() == 1) - return candidates[0]; + auto format_choices = [&](const Argument &arg) { + std::string f = indent + "allowed_values=["; + std::vector choices = std::get>(arg.limits); + f += "\"" + choices[0] + "\""; + for (int i = 1; i < choices.size(); ++i) { + f += ", \"" + choices[i] + "\""; + } + f += "],\n"; + return f; + }; - // return match if fully specified: - for (size_t i = 0; i < candidates.size(); ++i) - if (root == candidates[i]->id) - return candidates[i]; + auto format_output_template = [&](const std::string &id, const ArgType &type) { + std::string tmpl(id); + if (type == ImageOut) { + tmpl += ".mif"; + } else if (type == TracksOut) { + tmpl += ".tck"; + } else if (type == ArgFileOut) { + tmpl += ".txt"; + } + // TODO: Add special cases for file-out based on the 'id' where the extension + // is something else. + return tmpl; + }; - // check if there is only one *unique* candidate - const auto cid = candidates[0]->id; - if (std::all_of(++candidates.begin(), candidates.end(), [&cid](const Option *cand) { return cand->id == cid; })) - return candidates[0]; + auto format_output_templates = [&](const std::string &id, const Option &opt) { + if (opt.size() == 1) + return "\"" + format_output_template(id, opt[0].type) + "\""; + std::string tmpl = "("; + for (size_t i = 0; i < opt.size(); ++i) + tmpl += "\"" + format_output_template(id + MR::str(i), opt[i].type) + "\","; + tmpl += ")"; + return tmpl; + }; - // report something useful: - root = "several matches possible for option \"-" + root + "\": \"-" + candidates[0]->id; + auto format_arg_name = [&](const Argument &arg) { + std::string id = arg.id; + std::string arg_name; + if (id == "input" && (arg.type == ImageIn || arg.type == ArgFileIn)) + arg_name = "in_file"; + else if (id == "input" && arg.type == ArgDirectoryIn) + arg_name = "in_dir"; + else if (id == "output" && (arg.type == ImageOut || arg.type == ArgFileOut)) + arg_name = "out_file"; + else if (id == "output" && arg.type == ArgDirectoryOut) + arg_name = "out_dir"; + else + arg_name = escape_id(arg.id); + return arg_name; + }; - for (size_t i = 1; i < candidates.size(); ++i) - root += std::string("\", \"-") + candidates[i]->id + "\""; + auto format_argument = [&](const Argument &arg, int position, bool is_output = false) { + bool is_multi = (arg.flags & AllowMultiple) && (arg.type != ArgFileOut); + // Print name of field + std::string f = ""; + std::string arg_name = format_arg_name(arg); + f += base_indent + arg_name + ": "; + // Print type + std::string type = ""; + if (is_multi) { + if (is_output) + type += "list["; + else + type += "MultiInputObj["; + } + type += format_type(arg.type); + if (is_multi) { + type += "]"; + } + if (arg.flags & Optional) { + type += " | None"; + } + f += type; + if (is_output) + f += " = shell.outarg(\n"; + else + f += " = shell.arg(\n"; + // Print metadata fields + f += indent + "argstr=\"\",\n"; + f += indent + "position=" + std::to_string(position) + ",\n"; + if (arg.flags & Optional) { + if (arg.type == Boolean) + f += indent + "default=False,\n"; + else + f += indent + "default=None,\n"; + } + if (is_output) { + f += indent + "path_template=\"" + format_output_template(arg_name, arg.type) + "\",\n"; + } - throw Exception(root); -} + f += indent + "help=\"\"\"" + arg.desc + "\"\"\",\n"; -void sort_arguments(const std::vector &arguments) { - auto it = arguments.begin(); - while (it != arguments.end()) { - const size_t index = std::distance(arguments.begin(), it); - const Option *opt = match_option(*it); - if (opt != nullptr) { - if (it + opt->size() >= arguments.end()) { - throw Exception(std::string("not enough parameters to option \"-") + opt->id + "\""); - } + if (arg.type == Choice) { + f += format_choices(arg); + } + f += base_indent + ")\n"; + return f; + }; - std::vector option_args; - std::copy_n(it + 1, opt->size(), std::back_inserter(option_args)); - option.push_back(ParsedOption(opt, option_args, index)); - it += opt->size(); - } else { - argument.push_back(ParsedArgument(nullptr, nullptr, *it, index)); + auto format_option = [&](const Option &opt, bool is_output = false) { + // Print name of field + std::string f = base_indent + escape_id(opt.id) + ": "; + std::string type_string = format_option_type(opt); + bool is_multi = type_string.length() > 19 && type_string.substr(0, 19) == "MultiInputObj"; + if (is_output && !is_multi) { + type_string += "| bool | None"; + } else if (opt.flags & Optional && type_string != "bool" && type_string != "ty.Any") { + type_string += " | None"; + } + // Print type + f += type_string; + if (is_output) + f += " = shell.outarg(\n"; + else + f += " = shell.arg(\n"; + if (opt.flags & Optional) { + if (type_string == "bool") + f += indent + "default=False,\n"; + else + f += indent + "default=None,\n"; + } + // Print metadata fields + f += indent + "argstr= \"-" + opt.id + "\",\n"; + if (is_output) { + f += indent + "path_template=" + format_output_templates(escape_id(opt.id), opt) + ",\n"; + } + f += indent + "help=\"\"\"" + opt.desc + "\"\"\",\n"; + if (opt.size() == 1 && (opt[0].type == IntSeq || opt[0].type == FloatSeq)) { + f += indent + "sep=\",\",\n"; + } else if (opt.size() > 1) { + f += indent + "sep=\" \",\n"; + } + if (opt.size() == 1 && opt[0].type == Choice) { + f += format_choices(opt[0]); + } + f += base_indent + ")\n"; + return f; + }; + + auto option_is_output = [&](const Option &opt) { + for (size_t i = 0; i < opt.size(); ++i) { + if (opt[i].type == ImageOut || opt[i].type == ArgFileOut || opt[i].type == ArgDirectoryOut) + return true; } - ++it; + return false; + }; + + auto argument_is_output = [&](const Argument &arg) { + return arg.type == ImageOut || arg.type == ArgFileOut || arg.type == ArgDirectoryOut; + }; + + // Create actual class + s += "\n\n@shell.define\nclass " + name_string + "(shell.Task[\"" + name_string + ".Outputs\"]):\n"; + s += " \"\"\""; + // Add description + if (DESCRIPTION.size()) { + for (size_t i = 0; i < DESCRIPTION.size(); ++i) + s += base_indent + std::string(DESCRIPTION[i]) + "\n\n"; } -} -void parse_standard_options() { - if (!get_options("info").empty()) - log_level = std::max(log_level, 2); - if (!get_options("debug").empty()) - log_level = 3; - if (!get_options("quiet").empty()) - log_level = 0; - if (!get_options("force").empty()) { - WARN("existing output files will be overwritten"); - overwrite_files = true; + if (EXAMPLES.size()) { + s += "\n" + base_indent + "Example usages\n" + base_indent + "--------------\n\n"; + for (size_t i = 0; i < EXAMPLES.size(); ++i) { + s += "\n" + base_indent + EXAMPLES[i].title + ":\n\n"; + s += base_indent + "`$ " + EXAMPLES[i].code + "`\n\n"; + if (EXAMPLES[i].description.size()) + s += base_indent + EXAMPLES[i].description + "\n"; + s += "\n"; + } } -} -void verify_usage() { - if (AUTHOR.empty()) - throw Exception("No author specified for command " + std::string(NAME)); - if (SYNOPSIS.empty()) - throw Exception("No synopsis specified for command " + std::string(NAME)); -} + s += "\n" + base_indent + "References\n" + base_indent + "----------\n\n"; + for (size_t i = 0; i < REFERENCES.size(); ++i) + s += indent + REFERENCES[i] + "\n\n"; + s += indent + MRTRIX_CORE_REFERENCE + "\n\n"; -void parse_special_options() { - // special options are only accessible as the first argument - if (raw_arguments_list.size() != 1) - return; + s += "\n" + base_indent + "MRtrix\n" + base_indent + "------" + "\n\n" + base_indent + "Version:" + mrtrix_version + + ", built " + build_date + "\n\n" + base_indent + "Author: " + AUTHOR + "\n\n" + base_indent + + "Copyright: " + COPYRIGHT; + s += " \"\"\"\n"; + s += " executable = \"" + NAME + "\"\n"; - if (raw_arguments_list.front() == "__print_full_usage__") { - print(full_usage()); - throw 0; - } - if (raw_arguments_list.front() == "__print_usage_markdown__") { - print(markdown_usage()); - throw 0; + s += "\n" + base_indent + "# Arguments\n"; + + // Print out input spec + for (size_t i = 0; i < ARGUMENTS.size(); ++i) { + if (!argument_is_output(ARGUMENTS[i])) + s += format_argument(ARGUMENTS[i], i + 1); } - if (raw_arguments_list.front() == "__print_usage_rst__") { - print(restructured_text_usage()); - throw 0; + + s += "\n" + base_indent + "# Options\n"; + + std::vector group_names; + for (size_t i = 0; i < OPTIONS.size(); ++i) { + if (std::find(group_names.begin(), group_names.end(), OPTIONS[i].name) == group_names.end()) + group_names.push_back(OPTIONS[i].name); } - if (raw_arguments_list.front() == "__print_synopsis__") { - print(SYNOPSIS); - throw 0; + for (size_t i = 0; i < group_names.size(); ++i) { + size_t n = i; + while (OPTIONS[n].name != group_names[i]) + ++n; + if (OPTIONS[n].name != std::string("OPTIONS")) + s += std::string("\n") + base_indent + "# " + OPTIONS[n].name + ":\n"; + while (n < OPTIONS.size()) { + if (OPTIONS[n].name == group_names[i]) { + for (size_t o = 0; o < OPTIONS[n].size(); ++o) { + if (!option_is_output(OPTIONS[n][o])) { + s += format_option(OPTIONS[n][o]); + } + } + } + ++n; + } } -} -void parse() { - argument.clear(); - option.clear(); + s += "\n" + base_indent + "# Standard options\n"; + for (size_t i = 0; i < __standard_options.size(); ++i) + if (__standard_options[i].id != str("help") && __standard_options[i].id != str("version")) + s += format_option(__standard_options[i]); - sort_arguments(raw_arguments_list); + s += "\n" + base_indent + "class Outputs(shell.Outputs):\n"; - if (!get_options("help").empty()) { - print_help(); - throw 0; + // Add an additional indent + base_indent += " "; + indent += " "; + + int n_outputs = 0; + for (size_t i = 0; i < ARGUMENTS.size(); ++i) { + if (argument_is_output(ARGUMENTS[i])) { + s += format_argument(ARGUMENTS[i], i + 1, true); + n_outputs++; + } } - if (!get_options("version").empty()) { - print(version_string()); - throw 0; + + for (size_t i = 0; i < group_names.size(); ++i) { + size_t n = i; + while (OPTIONS[n].name != group_names[i]) + ++n; + while (n < OPTIONS.size()) { + if (OPTIONS[n].name == group_names[i]) { + for (size_t o = 0; o < OPTIONS[n].size(); ++o) { + if (option_is_output(OPTIONS[n][o])) { + s += format_option(OPTIONS[n][o], true); + n_outputs++; + } + } + } + ++n; + } } - size_t num_args_required = 0; - size_t num_optional_arguments = 0; + if (n_outputs == 0) + s += base_indent + "pass\n"; - ArgFlags flags = None; - for (size_t i = 0; i < ARGUMENTS.size(); ++i) { - if (ARGUMENTS[i].flags) { - if (flags && flags != ARGUMENTS[i].flags) - throw Exception("FIXME: all arguments declared optional() or allow_multiple() should have matching flags in " - "command-line syntax"); - flags = ARGUMENTS[i].flags; - ++num_optional_arguments; - if (!(flags & Optional)) - ++num_args_required; - } else - ++num_args_required; + return s; +} + +const Option *match_option(const char *arg) { + if (consume_dash(arg) && *arg && !isdigit(*arg) && *arg != '.') { + while (consume_dash(arg)) + ; + std::vector candidates; + std::string root(arg); + + for (size_t i = 0; i < OPTIONS.size(); ++i) + get_matches(candidates, OPTIONS[i], root); + get_matches(candidates, __standard_options, root); + + // no matches + if (candidates.size() == 0) + throw Exception(std::string("unknown option \"-") + root + "\""); + + // return match if unique: + if (candidates.size() == 1) + return candidates[0]; + + // return match if fully specified: + for (size_t i = 0; i < candidates.size(); ++i) + if (root == candidates[i]->id) + return candidates[i]; + + // check if there is only one *unique* candidate + const auto cid = candidates[0]->id; + if (std::all_of(++candidates.begin(), candidates.end(), [&cid](const Option *cand) { return cand->id == cid; })) + return candidates[0]; + + // report something useful: + root = "several matches possible for option \"-" + root + "\": \"-" + candidates[0]->id; + + for (size_t i = 1; i < candidates.size(); ++i) + root += std::string("\", \"-") + candidates[i]->id + "\""; + + throw Exception(root); } - if (option.empty() && argument.empty() && REQUIRES_AT_LEAST_ONE_ARGUMENT) { - print_help(); - throw 0; + const Option *match_option(std::string_view arg) { + auto no_dash_arg = without_leading_dash(arg); + if (arg.size() == no_dash_arg.size() || no_dash_arg.empty() || isdigit(no_dash_arg.front()) != 0 || + no_dash_arg.front() == '.') { + return nullptr; + } + + std::vector candidates; + std::string root(no_dash_arg); + + for (size_t i = 0; i < OPTIONS.size(); ++i) + get_matches(candidates, OPTIONS[i], root); + get_matches(candidates, __standard_options, root); + + // no matches + if (candidates.empty()) + throw Exception(std::string("unknown option \"-") + root + "\""); + + // return match if unique: + if (candidates.size() == 1) + return candidates[0]; + + // return match if fully specified: + for (size_t i = 0; i < candidates.size(); ++i) + if (root == candidates[i]->id) + return candidates[i]; + + // check if there is only one *unique* candidate + const auto cid = candidates[0]->id; + if (std::all_of(++candidates.begin(), candidates.end(), [&cid](const Option *cand) { return cand->id == cid; })) + return candidates[0]; + + // report something useful: + root = "several matches possible for option \"-" + root + "\": \"-" + candidates[0]->id; + + for (size_t i = 1; i < candidates.size(); ++i) + root += std::string("\", \"-") + candidates[i]->id + "\""; + + throw Exception(root); } - if (num_optional_arguments && num_args_required > argument.size()) - throw Exception("Expected at least " + str(num_args_required) + " arguments (" + str(argument.size()) + - " supplied)"); - - if (num_optional_arguments == 0 && num_args_required != argument.size()) { - Exception e("Expected exactly " + str(num_args_required) + " arguments (" + str(argument.size()) + " supplied)"); - std::string s = "Usage: " + NAME; - for (const auto &a : ARGUMENTS) - s += " " + std::string(a.id); - e.push_back(s); - s = "Yours: " + NAME; - for (const auto &a : argument) - s += " " + std::string(a); - e.push_back(s); - if (argument.size() > num_args_required) { - std::vector potential_options; - for (const auto &a : argument) { - for (const auto &og : OPTIONS) { - for (const auto &o : og) { - if (std::string(a) == std::string(o.id)) - potential_options.push_back("'-" + a + "'"); - } + void sort_arguments(const std::vector &arguments) { + auto it = arguments.begin(); + while (it != arguments.end()) { + const size_t index = std::distance(arguments.begin(), it); + const Option *opt = match_option(*it); + if (opt != nullptr) { + if (it + opt->size() >= arguments.end()) { + throw Exception(std::string("not enough parameters to option \"-") + opt->id + "\""); } + + std::vector option_args; + std::copy_n(it + 1, opt->size(), std::back_inserter(option_args)); + option.push_back(ParsedOption(opt, option_args, index)); + it += opt->size(); + } else { + argument.push_back(ParsedArgument(nullptr, nullptr, *it, index)); } - if (!potential_options.empty()) - e.push_back("(Did you mean " + join(potential_options, " or ") + "?)"); + ++it; } - throw e; } - size_t num_extra_arguments = argument.size() - num_args_required; - size_t num_arg_per_multi = num_optional_arguments ? num_extra_arguments / num_optional_arguments : 0; - if (num_arg_per_multi * num_optional_arguments != num_extra_arguments) - throw Exception("number of optional arguments provided are not equal for all arguments"); - if (!(flags & Optional)) - ++num_arg_per_multi; - - // assign arguments to their corresponding definitions: - for (size_t n = 0, index = 0, next = 0; n < argument.size(); ++n) { - if (n == next) { - if (n) - ++index; - if (ARGUMENTS[index].flags != None) - next = n + num_arg_per_multi; - else - ++next; + void parse_standard_options() { + if (!get_options("info").empty()) + log_level = std::max(log_level, 2); + if (!get_options("debug").empty()) + log_level = 3; + if (!get_options("quiet").empty()) + log_level = 0; + if (!get_options("force").empty()) { + WARN("existing output files will be overwritten"); + overwrite_files = true; } - argument[n].arg = &ARGUMENTS[index]; } - // check for multiple instances of options: - for (size_t i = 0; i < OPTIONS.size(); ++i) { - for (size_t j = 0; j < OPTIONS[i].size(); ++j) { - size_t count = 0; - for (size_t k = 0; k < option.size(); ++k) - if (option[k].opt == &OPTIONS[i][j]) - count++; + void verify_usage() { + if (AUTHOR.empty()) + throw Exception("No author specified for command " + std::string(NAME)); + if (SYNOPSIS.empty()) + throw Exception("No synopsis specified for command " + std::string(NAME)); + } - if (count < 1 && !(OPTIONS[i][j].flags & Optional)) - throw Exception(std::string("mandatory option \"-") + OPTIONS[i][j].id + "\" must be specified"); + void parse_special_options() { + // special options are only accessible as the first argument + if (raw_arguments_list.size() != 1) + return; - if (count > 1 && !(OPTIONS[i][j].flags & AllowMultiple)) - throw Exception(std::string("multiple instances of option \"-") + OPTIONS[i][j].id + "\" are not allowed"); + if (raw_arguments_list.front() == "__print_full_usage__") { + print(full_usage()); + throw 0; + } + if (raw_arguments_list.front() == "__print_usage_markdown__") { + print(markdown_usage()); + throw 0; + } + if (raw_arguments_list.front() == "__print_usage_rst__") { + print(restructured_text_usage()); + throw 0; + } + if (raw_arguments_list.front() == "__print_usage_pydra__") { + print(pydra_usage()); + throw 0; + } + if (raw_arguments_list.front() == "__print_synopsis__") { + print(SYNOPSIS); + throw 0; } } - parse_standard_options(); + void parse() { + argument.clear(); + option.clear(); - File::Config::init(); + sort_arguments(raw_arguments_list); - // CONF option: FailOnWarn - // CONF default: 0 (false) - // CONF A boolean value specifying whether MRtrix applications should - // CONF abort as soon as any (otherwise non-fatal) warning is issued. - fail_on_warn = File::Config::get_bool("FailOnWarn", false); - - // CONF option: TerminalColor - // CONF default: 1 (true) - // CONF A boolean value to indicate whether colours should be used in the terminal. - terminal_use_colour = File::Config::get_bool("TerminalColor", terminal_use_colour); - - // check for the existence of all specified input files (including optional ones that have been provided) - // if necessary, also check for pre-existence of any output files with known paths - // (if the output is e.g. given as a prefix, the argument should be flagged as type_text()) - for (const auto &i : argument) { - const std::string text = std::string(i); - if (i.arg->type == ArgFileIn || i.arg->type == TracksIn) { - if (!Path::exists(text)) - throw Exception("required input file \"" + text + "\" not found"); - if (!Path::is_file(text)) - throw Exception("required input \"" + text + "\" is not a file"); + if (!get_options("help").empty()) { + print_help(); + throw 0; } - if (i.arg->type == ArgDirectoryIn) { - if (!Path::exists(text)) - throw Exception("required input directory \"" + text + "\" not found"); - if (!Path::is_dir(text)) - throw Exception("required input \"" + text + "\" is not a directory"); + if (!get_options("version").empty()) { + print(version_string()); + throw 0; } - if (i.arg->type == ArgFileOut || i.arg->type == TracksOut) { - if (text.find_last_of(PATH_SEPARATORS) == text.size() - 1) - throw Exception("output path \"" + std::string(i) + - "\" is not a valid file path (ends with directory path separator)"); - check_overwrite(text); + + size_t num_args_required = 0; + size_t num_optional_arguments = 0; + + ArgFlags flags = None; + for (size_t i = 0; i < ARGUMENTS.size(); ++i) { + if (ARGUMENTS[i].flags) { + if (flags && flags != ARGUMENTS[i].flags) + throw Exception("FIXME: all arguments declared optional() or allow_multiple() should have matching flags in " + "command-line syntax"); + flags = ARGUMENTS[i].flags; + ++num_optional_arguments; + if (!(flags & Optional)) + ++num_args_required; + } else + ++num_args_required; } - if (i.arg->type == ArgDirectoryOut) - check_overwrite(text); - if (i.arg->type == TracksIn && !Path::has_suffix(text, ".tck")) - throw Exception("input file \"" + text + "\" is not a valid track file"); - if (i.arg->type == TracksOut && !Path::has_suffix(text, ".tck")) - throw Exception("output track file \"" + text + "\" must use the .tck suffix"); - } - for (const auto &i : option) { - for (size_t j = 0; j != i.opt->size(); ++j) { - const Argument &arg = i.opt->operator[](j); - const std::string text = std::string(i.args[j]); - if (arg.type == ArgFileIn || arg.type == TracksIn) { + + if (option.empty() && argument.empty() && REQUIRES_AT_LEAST_ONE_ARGUMENT) { + print_help(); + throw 0; + } + + if (num_optional_arguments && num_args_required > argument.size()) + throw Exception("Expected at least " + str(num_args_required) + " arguments (" + str(argument.size()) + + " supplied)"); + + if (num_optional_arguments == 0 && num_args_required != argument.size()) { + Exception e("Expected exactly " + str(num_args_required) + " arguments (" + str(argument.size()) + " supplied)"); + std::string s = "Usage: " + NAME; + for (const auto &a : ARGUMENTS) + s += " " + std::string(a.id); + e.push_back(s); + s = "Yours: " + NAME; + for (const auto &a : argument) + s += " " + std::string(a); + e.push_back(s); + if (argument.size() > num_args_required) { + std::vector potential_options; + for (const auto &a : argument) { + for (const auto &og : OPTIONS) { + for (const auto &o : og) { + if (std::string(a) == std::string(o.id)) + potential_options.push_back("'-" + a + "'"); + } + } + } + if (!potential_options.empty()) + e.push_back("(Did you mean " + join(potential_options, " or ") + "?)"); + } + throw e; + } + + size_t num_extra_arguments = argument.size() - num_args_required; + size_t num_arg_per_multi = num_optional_arguments ? num_extra_arguments / num_optional_arguments : 0; + if (num_arg_per_multi * num_optional_arguments != num_extra_arguments) + throw Exception("number of optional arguments provided are not equal for all arguments"); + if (!(flags & Optional)) + ++num_arg_per_multi; + + // assign arguments to their corresponding definitions: + for (size_t n = 0, index = 0, next = 0; n < argument.size(); ++n) { + if (n == next) { + if (n) + ++index; + if (ARGUMENTS[index].flags != None) + next = n + num_arg_per_multi; + else + ++next; + } + argument[n].arg = &ARGUMENTS[index]; + } + + // check for multiple instances of options: + for (size_t i = 0; i < OPTIONS.size(); ++i) { + for (size_t j = 0; j < OPTIONS[i].size(); ++j) { + size_t count = 0; + for (size_t k = 0; k < option.size(); ++k) + if (option[k].opt == &OPTIONS[i][j]) + count++; + + if (count < 1 && !(OPTIONS[i][j].flags & Optional)) + throw Exception(std::string("mandatory option \"-") + OPTIONS[i][j].id + "\" must be specified"); + + if (count > 1 && !(OPTIONS[i][j].flags & AllowMultiple)) + throw Exception(std::string("multiple instances of option \"-") + OPTIONS[i][j].id + "\" are not allowed"); + } + } + + parse_standard_options(); + + File::Config::init(); + + // CONF option: FailOnWarn + // CONF default: 0 (false) + // CONF A boolean value specifying whether MRtrix applications should + // CONF abort as soon as any (otherwise non-fatal) warning is issued. + fail_on_warn = File::Config::get_bool("FailOnWarn", false); + + // CONF option: TerminalColor + // CONF default: 1 (true) + // CONF A boolean value to indicate whether colours should be used in the terminal. + terminal_use_colour = File::Config::get_bool("TerminalColor", terminal_use_colour); + + // check for the existence of all specified input files (including optional ones that have been provided) + // if necessary, also check for pre-existence of any output files with known paths + // (if the output is e.g. given as a prefix, the argument should be flagged as type_text()) + for (const auto &i : argument) { + const std::string text = std::string(i); + if (i.arg->type == ArgFileIn || i.arg->type == TracksIn) { if (!Path::exists(text)) - throw Exception("input file \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" not found"); + throw Exception("required input file \"" + text + "\" not found"); if (!Path::is_file(text)) - throw Exception("input \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" is not a file"); + throw Exception("required input \"" + text + "\" is not a file"); } - if (arg.type == ArgDirectoryIn) { + if (i.arg->type == ArgDirectoryIn) { if (!Path::exists(text)) - throw Exception("input directory \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" not found"); + throw Exception("required input directory \"" + text + "\" not found"); if (!Path::is_dir(text)) - throw Exception("input \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" is not a directory"); + throw Exception("required input \"" + text + "\" is not a directory"); } - if (arg.type == ArgFileOut || arg.type == TracksOut) { + if (i.arg->type == ArgFileOut || i.arg->type == TracksOut) { if (text.find_last_of(PATH_SEPARATORS) == text.size() - 1) - throw Exception("output path \"" + text + "\" for option \"-" + std::string(i.opt->id) + + throw Exception("output path \"" + std::string(i) + "\" is not a valid file path (ends with directory path separator)"); check_overwrite(text); } - if (arg.type == ArgDirectoryOut) + if (i.arg->type == ArgDirectoryOut) check_overwrite(text); - if (arg.type == TracksIn && !Path::has_suffix(text, ".tck")) - throw Exception("input file \"" + text + "\" for option \"-" + std::string(i.opt->id) + - "\" is not a valid track file"); - if (arg.type == TracksOut && !Path::has_suffix(text, ".tck")) - throw Exception("output track file \"" + text + "\" for option \"-" + std::string(i.opt->id) + - "\" must use the .tck suffix"); + if (i.arg->type == TracksIn && !Path::has_suffix(text, ".tck")) + throw Exception("input file \"" + text + "\" is not a valid track file"); + if (i.arg->type == TracksOut && !Path::has_suffix(text, ".tck")) + throw Exception("output track file \"" + text + "\" must use the .tck suffix"); + } + for (const auto &i : option) { + for (size_t j = 0; j != i.opt->size(); ++j) { + const Argument &arg = i.opt->operator[](j); + const std::string text = std::string(i.args[j]); + if (arg.type == ArgFileIn || arg.type == TracksIn) { + if (!Path::exists(text)) + throw Exception("input file \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" not found"); + if (!Path::is_file(text)) + throw Exception("input \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" is not a file"); + } + if (arg.type == ArgDirectoryIn) { + if (!Path::exists(text)) + throw Exception("input directory \"" + text + "\" for option \"-" + std::string(i.opt->id) + + "\" not found"); + if (!Path::is_dir(text)) + throw Exception("input \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" is not a directory"); + } + if (arg.type == ArgFileOut || arg.type == TracksOut) { + if (text.find_last_of(PATH_SEPARATORS) == text.size() - 1) + throw Exception("output path \"" + text + "\" for option \"-" + std::string(i.opt->id) + + "\" is not a valid file path (ends with directory path separator)"); + check_overwrite(text); + } + if (arg.type == ArgDirectoryOut) + check_overwrite(text); + if (arg.type == TracksIn && !Path::has_suffix(text, ".tck")) + throw Exception("input file \"" + text + "\" for option \"-" + std::string(i.opt->id) + + "\" is not a valid track file"); + if (arg.type == TracksOut && !Path::has_suffix(text, ".tck")) + throw Exception("output track file \"" + text + "\" for option \"-" + std::string(i.opt->id) + + "\" must use the .tck suffix"); + } } - } - SignalHandler::init(); -} + SignalHandler::init(); + } -void init(int cmdline_argc, const char *const *cmdline_argv) { + void init(int cmdline_argc, const char *const *cmdline_argv) { #ifdef MRTRIX_WINDOWS - // force stderr to be unbuffered, and stdout to be line-buffered: - setvbuf(stderr, nullptr, _IONBF, 0); - setvbuf(stdout, nullptr, _IOLBF, 0); + // force stderr to be unbuffered, and stdout to be line-buffered: + setvbuf(stderr, nullptr, _IONBF, 0); + setvbuf(stdout, nullptr, _IOLBF, 0); #endif - terminal_use_colour = !ProgressBar::set_update_method(); + terminal_use_colour = !ProgressBar::set_update_method(); - raw_arguments_list = std::vector(cmdline_argv, cmdline_argv + cmdline_argc); - NAME = Path::basename(raw_arguments_list.front()); - raw_arguments_list.erase(raw_arguments_list.begin()); + raw_arguments_list = std::vector(cmdline_argv, cmdline_argv + cmdline_argc); + NAME = Path::basename(raw_arguments_list.front()); + raw_arguments_list.erase(raw_arguments_list.begin()); #ifdef MRTRIX_WINDOWS - if (Path::has_suffix(NAME, ".exe")) - NAME.erase(NAME.size() - 4); + if (Path::has_suffix(NAME, ".exe")) + NAME.erase(NAME.size() - 4); #endif - auto argv_quoted = [](const std::string &s) -> std::string { - for (size_t i = 0; i != s.size(); ++i) { - if (!(isalnum(s[i]) || s[i] == '.' || s[i] == '_' || s[i] == '-' || s[i] == '/')) { - std::string escaped_string("\'"); - for (auto c : s) { - switch (c) { - case '\'': - escaped_string.append("\\\'"); - break; - case '\\': - escaped_string.append("\\\\"); - break; - default: - escaped_string.push_back(c); - break; + auto argv_quoted = [](const std::string &s) -> std::string { + for (size_t i = 0; i != s.size(); ++i) { + if (!(isalnum(s[i]) || s[i] == '.' || s[i] == '_' || s[i] == '-' || s[i] == '/')) { + std::string escaped_string("\'"); + for (auto c : s) { + switch (c) { + case '\'': + escaped_string.append("\\\'"); + break; + case '\\': + escaped_string.append("\\\\"); + break; + default: + escaped_string.push_back(c); + break; + } } + escaped_string.push_back('\''); + return escaped_string; } - escaped_string.push_back('\''); - return escaped_string; } - } - return s; - }; - command_history_string = cmdline_argv[0]; - for (const auto &a : raw_arguments_list) - command_history_string += std::string(" ") + argv_quoted(a); - command_history_string += std::string(" (version=") + mrtrix_version; - if (project_version) - command_history_string += std::string(", project=") + project_version; - command_history_string += ")"; - - std::locale::global(std::locale::classic()); - std::setlocale(LC_ALL, "C"); - - srand(time(nullptr)); -} - -const std::vector get_options(const std::string &name) { - assert(!name.empty()); - assert(name[0] != '-'); - std::vector matches; - for (size_t i = 0; i < option.size(); ++i) { - assert(option[i].opt); - if (option[i].opt->is(name)) - matches.push_back({option[i].opt, option[i].args, option[i].index}); + return s; + }; + command_history_string = cmdline_argv[0]; + for (const auto &a : raw_arguments_list) + command_history_string += std::string(" ") + argv_quoted(a); + command_history_string += std::string(" (version=") + mrtrix_version; + if (project_version) + command_history_string += std::string(", project=") + project_version; + command_history_string += ")"; + + std::locale::global(std::locale::classic()); + std::setlocale(LC_ALL, "C"); + + srand(time(nullptr)); } - return matches; -} -int64_t App::ParsedArgument::as_int() const { - if (arg->type == Integer) { - - // Check to see if there are any alpha characters in here - // - If a single character at the end, use as integer multiplier - // - Unless there's a dot point before the multiplier; in which case, - // parse the number as a float, multiply, then cast to integer - // - If a single 'e' or 'E' in the middle, parse as float and convert to integer - size_t alpha_count = 0; - bool alpha_is_last = false; - bool contains_dotpoint = false; - char alpha_char = 0; - for (const char &c : p) { - if (std::isalpha(c) != 0) { - ++alpha_count; - alpha_is_last = true; - alpha_char = c; - } else { - alpha_is_last = false; - } - if (c == '.') - contains_dotpoint = true; + const std::vector get_options(const std::string &name) { + assert(!name.empty()); + assert(name[0] != '-'); + std::vector matches; + for (size_t i = 0; i < option.size(); ++i) { + assert(option[i].opt); + if (option[i].opt->is(name)) + matches.push_back({option[i].opt, option[i].args, option[i].index}); } - if (alpha_count > 1) - throw Exception("error converting string " + str(p) + " to integer: too many letters"); - int64_t retval = 0; - if (alpha_count) { - if (alpha_is_last) { - std::string num(p); - const char postfix = num.back(); - num.pop_back(); - int64_t multiplier = 1.0; - switch (postfix) { - case 'k': - case 'K': - multiplier = 1000; - break; - case 'm': - case 'M': - multiplier = 1000000; - break; - case 'b': - case 'B': - multiplier = 1000000000; - break; - case 't': - case 'T': - multiplier = 1000000000000; - break; - default: - throw Exception("error converting string " + str(p) + " to integer: unexpected postfix \'" + postfix + "\'"); + return matches; + } + + int64_t App::ParsedArgument::as_int() const { + if (arg->type == Integer) { + + // Check to see if there are any alpha characters in here + // - If a single character at the end, use as integer multiplier + // - Unless there's a dot point before the multiplier; in which case, + // parse the number as a float, multiply, then cast to integer + // - If a single 'e' or 'E' in the middle, parse as float and convert to integer + size_t alpha_count = 0; + bool alpha_is_last = false; + bool contains_dotpoint = false; + char alpha_char = 0; + for (const char &c : p) { + if (std::isalpha(c) != 0) { + ++alpha_count; + alpha_is_last = true; + alpha_char = c; + } else { + alpha_is_last = false; } - if (contains_dotpoint) { - const default_type prefix = to(num); - retval = std::round(prefix * default_type(multiplier)); + if (c == '.') + contains_dotpoint = true; + } + if (alpha_count > 1) + throw Exception("error converting string " + str(p) + " to integer: too many letters"); + int64_t retval = 0; + if (alpha_count) { + if (alpha_is_last) { + std::string num(p); + const char postfix = num.back(); + num.pop_back(); + int64_t multiplier = 1.0; + switch (postfix) { + case 'k': + case 'K': + multiplier = 1000; + break; + case 'm': + case 'M': + multiplier = 1000000; + break; + case 'b': + case 'B': + multiplier = 1000000000; + break; + case 't': + case 'T': + multiplier = 1000000000000; + break; + default: + throw Exception("error converting string " + str(p) + " to integer: unexpected postfix \'" + postfix + + "\'"); + } + if (contains_dotpoint) { + const default_type prefix = to(num); + retval = std::round(prefix * default_type(multiplier)); + } else { + retval = to(num) * multiplier; + } + } else if (alpha_char == 'e' || alpha_char == 'E') { + const default_type as_float = to(p); + retval = std::round(as_float); } else { - retval = to(num) * multiplier; + throw Exception("error converting string " + str(p) + " to integer: unexpected character"); } - } else if (alpha_char == 'e' || alpha_char == 'E') { - const default_type as_float = to(p); - retval = std::round(as_float); } else { - throw Exception("error converting string " + str(p) + " to integer: unexpected character"); + retval = to(p); } - } else { - retval = to(p); + + const auto range = std::get(arg->limits); + if (retval < range.min || retval > range.max) { + std::string msg("value supplied for "); + if (opt) + msg += std::string("option \"") + opt->id; + else + msg += std::string("argument \"") + arg->id; + msg += "\" is out of bounds (valid range: " + str(range.min) + " to " + str(range.max) + + ", value supplied: " + str(retval) + ")"; + throw Exception(msg); + } + return retval; + } + + if (arg->type == Choice) { + std::string selection = lowercase(p); + const auto &choices = std::get>(arg->limits); + auto it = std::find(choices.begin(), choices.end(), selection); + if (it == choices.end()) { + std::string msg = std::string("unexpected value supplied for "); + if (opt != nullptr) + msg += std::string("option \"") + opt->id; + else + msg += std::string("argument \"") + arg->id; + msg += std::string("\" (received \"" + std::string(p) + "\"; valid choices are: ") + join(choices, ", ") + ")"; + throw Exception(msg); + } + return static_cast(std::distance(choices.begin(), it)); } + assert(0); + return (0); + } - const auto range = std::get(arg->limits); + default_type App::ParsedArgument::as_float() const { + assert(arg->type == Float); + const default_type retval = to(p); + const auto range = std::get(arg->limits); if (retval < range.min || retval > range.max) { std::string msg("value supplied for "); if (opt) @@ -1282,139 +1762,103 @@ int64_t App::ParsedArgument::as_int() const { ", value supplied: " + str(retval) + ")"; throw Exception(msg); } + return retval; } - if (arg->type == Choice) { - std::string selection = lowercase(p); - const auto &choices = std::get>(arg->limits); - auto it = std::find(choices.begin(), choices.end(), selection); - if (it == choices.end()) { - std::string msg = std::string("unexpected value supplied for "); - if (opt != nullptr) - msg += std::string("option \"") + opt->id; - else - msg += std::string("argument \"") + arg->id; - msg += std::string("\" (received \"" + std::string(p) + "\"; valid choices are: ") + join(choices, ", ") + ")"; - throw Exception(msg); + std::vector ParsedArgument::as_sequence_int() const { + assert(arg->type == IntSeq); + try { + return parse_ints(p); + } catch (Exception &e) { + error(e); } - return static_cast(std::distance(choices.begin(), it)); + return std::vector(); } - assert(0); - return (0); -} -default_type App::ParsedArgument::as_float() const { - assert(arg->type == Float); - const default_type retval = to(p); - const auto range = std::get(arg->limits); - if (retval < range.min || retval > range.max) { - std::string msg("value supplied for "); - if (opt) - msg += std::string("option \"") + opt->id; - else - msg += std::string("argument \"") + arg->id; - msg += "\" is out of bounds (valid range: " + str(range.min) + " to " + str(range.max) + - ", value supplied: " + str(retval) + ")"; - throw Exception(msg); + std::vector ParsedArgument::as_sequence_uint() const { + assert(arg->type == IntSeq); + try { + return parse_ints(p); + } catch (Exception &e) { + error(e); + } + return std::vector(); } - return retval; -} - -std::vector ParsedArgument::as_sequence_int() const { - assert(arg->type == IntSeq); - try { - return parse_ints(p); - } catch (Exception &e) { - error(e); + std::vector ParsedArgument::as_sequence_float() const { + assert(arg->type == FloatSeq); + try { + return parse_floats(p); + } catch (Exception &e) { + error(e); + } + return std::vector(); } - return std::vector(); -} -std::vector ParsedArgument::as_sequence_uint() const { - assert(arg->type == IntSeq); - try { - return parse_ints(p); - } catch (Exception &e) { - error(e); + ParsedArgument::ParsedArgument(const Option *option, const Argument *argument, std::string text, size_t index) + : opt(option), arg(argument), p(std::move(text)), index_(index) { + assert(!p.empty()); } - return std::vector(); -} -std::vector ParsedArgument::as_sequence_float() const { - assert(arg->type == FloatSeq); - try { - return parse_floats(p); - } catch (Exception &e) { - error(e); + void ParsedArgument::error(Exception & e) const { + std::string msg("error parsing token \""); + msg += p; + if (opt != nullptr) + msg += std::string("\" for option \"") + opt->id + "\""; + else + msg += std::string("\" for argument \"") + arg->id + "\""; + throw Exception(e, msg); } - return std::vector(); -} - -ParsedArgument::ParsedArgument(const Option *option, const Argument *argument, std::string text, size_t index) - : opt(option), arg(argument), p(std::move(text)), index_(index) { - assert(!p.empty()); -} -void ParsedArgument::error(Exception &e) const { - std::string msg("error parsing token \""); - msg += p; - if (opt != nullptr) - msg += std::string("\" for option \"") + opt->id + "\""; - else - msg += std::string("\" for argument \"") + arg->id + "\""; - throw Exception(e, msg); -} - -void check_overwrite(const std::string &name) { - if (Path::exists(name) && !overwrite_files) { - if (check_overwrite_files_func != nullptr) - check_overwrite_files_func(name); - else - throw Exception("output path \"" + name + "\" already exists (use -force option to force overwrite)"); + void check_overwrite(const std::string &name) { + if (Path::exists(name) && !overwrite_files) { + if (check_overwrite_files_func != nullptr) + check_overwrite_files_func(name); + else + throw Exception("output path \"" + name + "\" already exists (use -force option to force overwrite)"); + } } -} -ParsedOption::ParsedOption(const Option *option, const std::vector &arguments, size_t i) - : opt(option), args(arguments), index(i) { - for (size_t i = 0; i != option->size(); ++i) { - const auto &p = arguments[i]; - if (!starts_with_dash(p)) - continue; - if (((*option)[i].type == ImageIn || (*option)[i].type == ImageOut) && is_dash(arguments[i])) - continue; - if ((*option)[i].type == Integer || (*option)[i].type == Float || (*option)[i].type == IntSeq || - (*option)[i].type == FloatSeq || (*option)[i].type == Various) - continue; - WARN(std::string("Value \"") + arguments[i] + "\" is being used as " + - ((option->size() == 1) ? "the expected argument " - : ("one of the " + str(option->size()) + " expected arguments ")) + - "for option \"-" + option->id + "\", yet this itself looks like a separate command-line option; " + - "the requisite input" + ((option->size() == 1) ? " " : "s ") + "to command-line option \"-" + option->id + - "\" may have been erroneously omitted, which may cause " + "other command-line parsing errors"); + ParsedOption::ParsedOption(const Option *option, const std::vector &arguments, size_t i) + : opt(option), args(arguments), index(i) { + for (size_t i = 0; i != option->size(); ++i) { + const auto &p = arguments[i]; + if (!starts_with_dash(p)) + continue; + if (((*option)[i].type == ImageIn || (*option)[i].type == ImageOut) && is_dash(arguments[i])) + continue; + if ((*option)[i].type == Integer || (*option)[i].type == Float || (*option)[i].type == IntSeq || + (*option)[i].type == FloatSeq || (*option)[i].type == Various) + continue; + WARN(std::string("Value \"") + arguments[i] + "\" is being used as " + + ((option->size() == 1) ? "the expected argument " + : ("one of the " + str(option->size()) + " expected arguments ")) + + "for option \"-" + option->id + "\", yet this itself looks like a separate command-line option; " + + "the requisite input" + ((option->size() == 1) ? " " : "s ") + "to command-line option \"-" + option->id + + "\" may have been erroneously omitted, which may cause " + "other command-line parsing errors"); + } } -} -ParsedArgument ParsedOption::operator[](size_t num) const { - assert(num < opt->size()); - return ParsedArgument(opt, &(*opt)[num], args[num], index + num + 1); -} + ParsedArgument ParsedOption::operator[](size_t num) const { + assert(num < opt->size()); + return ParsedArgument(opt, &(*opt)[num], args[num], index + num + 1); + } -bool ParsedOption::operator==(const char *match) const { - const std::string name = lowercase(match); - return name == opt->id; -} + bool ParsedOption::operator==(const char *match) const { + const std::string name = lowercase(match); + return name == opt->id; + } -std::string operator+(const char *left, const ParsedArgument &right) { - std::string retval(left); - retval += std::string(right); - return retval; -} + std::string operator+(const char *left, const ParsedArgument &right) { + std::string retval(left); + retval += std::string(right); + return retval; + } -std::ostream &operator<<(std::ostream &stream, const ParsedArgument &arg) { - stream << std::string(arg); - return stream; -} + std::ostream &operator<<(std::ostream &stream, const ParsedArgument &arg) { + stream << std::string(arg); + return stream; + } } // namespace MR::App diff --git a/python/mrtrix3/app.py b/python/mrtrix3/app.py index a7ceb74499..d41eba8740 100644 --- a/python/mrtrix3/app.py +++ b/python/mrtrix3/app.py @@ -1481,6 +1481,243 @@ def print_group_options(group): alg, '__print_usage_rst__']) + def print_usage_pydra(self): + + if self._subparsers: + + if len(sys.argv) == 3: + for alg in self._subparsers._group_actions[0].choices: + if alg == sys.argv[-2]: + self._subparsers._group_actions[0].choices[alg].print_usage_pydra() + return + self.error('Invalid subparser nominated: ' + sys.argv[-2]) + assert len(sys.argv) == 2 + sys.stdout.write(",".join(self._subparsers._group_actions[0].choices)) + sys.stdout.flush() + return + + def get_arg_metadata(arg): + kwds = { + "help": arg.help, + } + if arg.choices: + kwds["allowed_values"] = list(arg.choices) + return kwds + + def parse_type(type_, optional: bool = False): + if type_ is str or type_ is None: + type_str = "str" + elif isinstance(type_, Parser.Various): + type_str = "typing.Any" + elif isinstance(type_, Parser.Bool): + type_str = "bool" + elif type(type_).__name__ == "IntBounded": + type_str = "int" + elif type(type_).__name__ == "FloatBounded": + type_str = "float" + elif isinstance(type_, Parser.FileIn): + type_str = "File" + elif isinstance(type_, Parser.FileOut): + type_str = "File" + elif isinstance(type_, Parser.DirectoryIn): + type_str = "Directory" + elif isinstance(type_, Parser.DirectoryOut): + type_str = "Directory" + elif isinstance(type_, Parser.SequenceDirectoryOut): + type_str = "typing.List[Directory]" + elif isinstance(type_, Parser.ImageIn): + type_str = "ImageIn" + elif isinstance(type_, Parser.ImageOut): + type_str = "ImageOut" + elif isinstance(type_, Parser.SequenceInt): + type_str = "typing.List[int]" + elif isinstance(type_, Parser.SequenceFloat): + type_str = "typing.List[float]" + elif isinstance(type_, Parser.TracksIn): + type_str = "Tracks" + elif isinstance(type_, Parser.TracksOut): + type_str = "Tracks" + else: + raise ValueError("Unrecognized type: " + str(type_)) + if optional: + type_str += " | None" + return type_str + + def escape_id(id_: str) -> str: + if id_ == "input": + escaped = "in_file" + elif id_ == "output": + escaped = "out_file" + elif id_ in list(PYTHON_KEYWORDS) + ["container", "image", "container_xargs"]: + escaped = id_ + "_" + else: + escaped = id_ + escaped = escaped.replace(".", '_') + return escaped + + def is_output(arg_option) -> bool: + tp = arg_option.type + if isinstance(tp, (Parser.FileOut, Parser.DirectoryOut, Parser.ImageOut, Parser.SequenceDirectoryOut)): + return True + return hasattr(tp, "_legacytypestring") and tp._legacytypestring().endswith("OUT") + + inputs = [] + outputs = [] + input_names = [a.dest for a in self._positionals._group_actions] + for pos, arg in enumerate(self._positionals._group_actions, start=1): + kwds = { + "position": pos, + "argstr": "", + "help": arg.help, + } + if arg.choices: + kwds["allowed_values"] = list(arg.choices) + arg_id = escape_id(arg.dest) + type_ = parse_type(arg.type) + if is_output(arg): + if isinstance(arg.type, Parser.ImageOut): + ext = ".mif" + elif isinstance(arg.type, Parser.FileOut): + ext = ".txt" + else: + ext = "" + kwds["path_template"] = arg_id + ext + (outputs if is_output(arg) else inputs).append( + ( + arg_id, + type_, + kwds, + ) + ) + for group in reversed(self._action_groups): + for option in group._group_actions: + if option.dest in input_names: + continue + if isinstance(option, argparse._StoreTrueAction): + assert option.type is None + type_ = "bool" + else: + type_ = parse_type(option.type, optional=True) + if isinstance(option, argparse._AppendAction): + if is_output(option): + type_ = "list" + else: + type_ = "MultiInputObj" + type_ += f"[{type_}]" + kwds = get_arg_metadata(option) + kwds["argstr"] = "-" + option.dest + if type_ == "bool": + kwds["default"] = False + else: + kwds["default"] = None + if is_output(option): + if isinstance(option.type, Parser.ImageOut): + ext = ".mif" + elif isinstance(option.type, Parser.FileOut): + ext = ".txt" + else: + ext = "" + kwds["path_template"] = escape_id(option.dest) + ext + (outputs if is_output(option) else inputs).append( + ( + escape_id(option.dest), + type_, + kwds, + ) + ) + # Replace # escapes + inputs_str = "" + outputs_str = "" + indent = " " + md_indent = indent + " " + for inpt_name, type_, kwds in inputs: + inputs_str += f"{indent}{inpt_name}: {type_} = shell.arg(\n{indent} " + inputs_str += f"\n{md_indent}".join(f"{k}={v!r}," for k, v in kwds.items()) + inputs_str += f"\n{indent})\n" + for outpt_name, type_, kwds in outputs: + outputs_str += f"{indent} {outpt_name}: {type_} = shell.outarg(\n{indent} " + outputs_str += f"\n{md_indent} ".join(f"{k}={v!r}," for k, v in kwds.items()) + outputs_str += f"\n{indent} )\n" + + def cmd_to_task_name(cmd_name: str) -> str: + """Get Task class name from cmd name""" + if cmd_name == "population_template": + return "PopulationTemplate" + task_name = cmd_name.replace(" ", "_") + if task_name[0] == "5": + task_name = "five" + task_name[1:] + cmd_prefixes = [ + "fivett", "afd", "amp", "connectome", "dcm", "dir", "dwi", + "fixel", "fod", "label", "mask", "mesh", "mr", "mt", + "peaks", "response", "sh", "tck", "transform", "tsf", "voxel", "vector" + ] + # convert to PascalCase + task_name = "".join( + g.capitalize() + for g in re.match(rf"({'|'.join(cmd_prefixes)})(2?)([^_]+)(_?)(.*)", task_name).groups() + ) + return task_name + + task_name = cmd_to_task_name(self.prog) + + if self._mutually_exclusive_option_groups: + xor = [] + for group, required in self._mutually_exclusive_option_groups: + xor_set = list(group) + if not required: + xor_set.append(None) + xor.append(xor_set) + xor_str = f"(xor={xor!r})" + else: + xor_str = "" + + text = ( + "# Auto-generated by mrtrix3/app.py:print_usage_pydra()\n\n" + "import typing\n" + "from pathlib import Path # noqa: F401\n" + "from fileformats.generic import FsObject, File, Directory # noqa: F401\n" + "from fileformats.medimage_mrtrix3 import Tracks, ImageIn, ImageOut # noqa: F401\n" + "from pydra.utils.typing import MultiInputObj\n" + "from pydra.compose import shell\n" + ) + + text += f"\n\n@shell.define{xor_str}\nclass {task_name}(shell.Task[\"{task_name}.Outputs\"]):\n" + indent = " " + text += indent + "\"\"\"\n" + text += indent + (self.description if self.description else "").replace("\n", "\n ") + "\n" + text += indent + "References\n" + text += indent + "----------\n\n" + for ref in self._citation_list: + ref_text = indent + "* " + if ref[0]: + ref_text += ref[0] + ': ' + ref_text += ref[1] + text += ref_text + '\n\n' + text += indent + _MRTRIX3_CORE_REFERENCE.replace("\n", "\n ") + '\n\n' + text += indent + '--------------\n\n\n\n' + text += indent + '**Author:** ' + self._author + '\n\n' + text += indent + '**Copyright:** ' + self._copyright.replace("\n", "\n ") + '\n\n' + text += indent + "\"\"\"\n" + if " " in self.prog: + executable = tuple(self.prog.split(" ")) + else: + executable = self.prog + text += f" executable={executable!r}\n\n" + text += inputs_str + if outputs_str: + text += f"\n\n{indent}class Outputs(shell.Outputs):\n" + text += outputs_str + + if HAVE_BLACK: + try: + text = black.format_file_contents( + text, fast=False, mode=black.FileMode() + ) + except black.parsing.InvalidInput: + pass + sys.stdout.write(text) + sys.stdout.flush() + def print_version(self): text = f'== {self.prog} {self._git_version if self._is_project else version.VERSION} ==\n' if self._is_project: diff --git a/python/mrtrix3/commands/population_template/usage.py b/python/mrtrix3/commands/population_template/usage.py index 847b6e6bcd..78d02f2b9d 100644 --- a/python/mrtrix3/commands/population_template/usage.py +++ b/python/mrtrix3/commands/population_template/usage.py @@ -32,16 +32,7 @@ REGISTRATION_MODES -class SequenceDirectoryOut(app.Parser.CustomTypeBase): - def __call__(self, input_value): - return [app.Parser.make_userpath_object(app.Parser._UserDirOutPathExtras, item) # pylint: disable=protected-access \ - for item in input_value.split(',')] - @staticmethod - def _legacytypestring(): - return 'SEQDIROUT' - @staticmethod - def _metavar(): - return 'directory_list' + @@ -230,7 +221,7 @@ def usage(cmdline): #pylint: disable=unused-variable help='Output a directory containing warps from each input to the template.' ' If the folder does not exist it will be created') options.add_argument('-transformed_dir', - type=SequenceDirectoryOut(), + type=app.Parser.SequenceDirectoryOut(), help='Output a directory containing the input images transformed to the template.' ' If the folder does not exist it will be created.' ' For multi-contrast registration,' From 121f87cd3a92b738d655c2784e28b033b7bfcdb9 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Thu, 21 Aug 2025 14:55:52 +1000 Subject: [PATCH 02/27] deleted pydra package GHA workflow (now generated in https://github.com/nipype/pydra-tasks-mrtrix3 package) --- .github/workflows/package-pydra.yml | 243 ---------------------------- 1 file changed, 243 deletions(-) delete mode 100644 .github/workflows/package-pydra.yml diff --git a/.github/workflows/package-pydra.yml b/.github/workflows/package-pydra.yml deleted file mode 100644 index 9018bdfe86..0000000000 --- a/.github/workflows/package-pydra.yml +++ /dev/null @@ -1,243 +0,0 @@ -#This workflow will install Python dependencies, run tests and lint with a variety of Python versions -# For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions - -# For deployment, it will be necessary to create a PyPI API token and store it as a secret -# https://docs.github.com/en/actions/reference/encrypted-secrets - -name: Pydra - -on: - push: - branches: [ master ] - tags: [ '*' ] - pull_request: - branches: - - master - - dev - -jobs: - - generate-pydra: - - runs-on: ubuntu-latest - - env: - CFLAGS: -Werror - QT_SELECT: qt6 - SCCACHE_GHA_ENABLED: "true" - SCCACHE_CACHE_SIZE: "2G" - - steps: - - uses: actions/checkout@v1 - with: - submodules: true - - - name: Determine MRtrix3 version - run: echo "MRTRIX3_VERSION=$(git describe --tags --abbrev=0)" >> $GITHUB_ENV - - - name: install dependencies - run: | - sudo apt-get update - sudo apt-get install clang qt6-base-dev libglvnd-dev libeigen3-dev zlib1g-dev libfftw3-dev ninja-build - - - name: Run sccache-cache - uses: mozilla-actions/sccache-action@v0.0.3 - - - name: Get CMake - uses: lukka/get-cmake@latest - with: - cmakeVersion: '3.16.3' - - - name: Print CMake version - run: cmake --version - - - name: configure - run: > - cmake - -B build - -G Ninja - -D CMAKE_BUILD_TYPE=Release - -D MRTRIX_BUILD_TESTS=ON - -D MRTRIX_STL_DEBUGGING=ON - -D MRTRIX_WARNINGS_AS_ERRORS=ON - -D CMAKE_C_COMPILER=clang - -D CMAKE_CXX_COMPILER=clang++ - -D CMAKE_INSTALL_PREFIX=$(pwd)/install - - - name: Build Mrtrix - run: cmake --build build - - - name: Install Mrtrix - run: cmake --install build - - - name: Set PATH Variable - run: echo "PATH=$PATH:$(pwd)/install/bin" >> $GITHUB_ENV - - - name: Set LD_LIBRARY_PATH Variable - run: echo "LD_LIBRARY_PATH=$(pwd)/install/lib" >> $GITHUB_ENV - - - name: Set up Python - uses: actions/setup-python@v2 - - - name: Install Python build dependencies - run: | - python -m pip install --upgrade pip - - - name: Install pydra-auto-gen requirements - run: | - pip install -e interfaces/pydra/fileformats-medimage-mrtrix3 - pip install -e interfaces/pydra/fileformats-medimage-mrtrix3-extras - pip install -e interfaces/pydra/src - pip install -r interfaces/pydra/requirements.txt - - - name: Generate task specifications - run: > - ./interfaces/pydra/generate.py - $(pwd)/install/bin - $(pwd)/interfaces/pydra/src - $MRTRIX3_VERSION - --log-errors - --latest - - - name: Upload auto-gen pydra - uses: actions/upload-artifact@v2 - with: - name: AutoGen - path: interfaces/pydra/src/pydra/tasks/mrtrix3/v3_0 - - devcheck: - needs: [generate-pydra] - runs-on: ubuntu-latest - strategy: - matrix: - python-version: [3.8, 3.11] # Check oldest and newest versions - pip-flags: ['', '--editable'] - pydra: - - 'pydra' - - '--editable git+https://github.com/nipype/pydra.git#egg=pydra' - - steps: - - uses: actions/checkout@v2 - - - name: Download auto-gen pydra - uses: actions/download-artifact@v2 - with: - name: AutoGen - path: interfaces/pydra/src/pydra/tasks/mrtrix3/v3_0 - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 - with: - python-version: ${{ matrix.python-version }} - - - name: Install build dependencies - run: | - python -m pip install --upgrade pip - - - name: Install Pydra - run: | - pip install ${{ matrix.pydra }} - python -c "import pydra as m; print(f'{m.__name__} {m.__version__} @ {m.__file__}')" - - - name: Install task package - run: | - pip install ${{ matrix.pip-flags }} "interfaces/pydra/fileformats-medimage-mrtrix3" - pip install ${{ matrix.pip-flags }} "interfaces/pydra/src[dev]" - python -c "import pydra.tasks.mrtrix3 as m; print(f'{m.__name__} {m.__version__} @ {m.__file__}')" - python -c "import pydra.tasks.mrtrix3.v3_0" - python -c "import pydra as m; print(f'{m.__name__} {m.__version__} @ {m.__file__}')" - - # test: - # needs: [generate] - # runs-on: ubuntu-latest - # strategy: - # matrix: - # python-version: [3.8, 3.11] - # defaults: - # run: - # shell: bash -l {0} - # steps: - # - uses: actions/checkout@v2 - # - name: Install Minconda - # uses: conda-incubator/setup-miniconda@v2 - # with: - # auto-activate-base: true - # activate-environment: "" - # - name: Install MRtrix via Conda - # run: | - # conda install -c mrtrix3 mrtrix3 - # mrconvert --version - # - name: Set up Python ${{ matrix.python-version }} - # uses: actions/setup-python@v2 - # with: - # python-version: ${{ matrix.python-version }} - # - name: Install build dependencies - # run: | - # python -m pip install --upgrade pip - # - name: Install task package - # run: | - # pip install interfaces/pydra/fileformats-medimage-mrtrix interfaces/pydra/fileformats-medimage-mrtrix-extras 'interfaces/pydra/src[test]' - # python -c "import pydra.tasks.mrtrix3 as m; print(f'{m.__name__} {m.__version__} @ {m.__file__}')" - # python -c "import pydra as m; print(f'{m.__name__} {m.__version__} @ {m.__file__}')" - # - name: Test with pytest - # run: | - # pytest -sv --doctest-modules interfaces/src/pydra/tasks/mrtrix3 \ - # --cov pydra.tasks.mrtrix3 --cov-report xml - # - uses: codecov/codecov-action@v1 - # if: ${{ always() }} - - - deploy: - needs: [devcheck] # , test] - runs-on: ubuntu-latest - strategy: - matrix: - python-version: [3.11] - steps: - - uses: actions/checkout@v2 - - - name: Download auto-gen pydra - uses: actions/download-artifact@v2 - with: - name: AutoGen - path: interfaces/pydra/src/pydra/tasks/mrtrix3/v3_0 - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 - with: - python-version: ${{ matrix.python-version }} - - - name: Install build tools - run: python -m pip install --upgrade pip twine build - - - name: Build source and wheel distributions - working-directory: ./interfaces/pydra/src - run: python -m build - - - name: Check distributions - run: twine check interfaces/pydra/src/dist/* - - - name: Upload sdist - uses: actions/upload-artifact@v2 - with: - name: SDist - path: interfaces/pydra/src/dist/*.tar.gz - - # Deploy on tags if PYPI_API_TOKEN is defined in the repository secrets. - # Secrets are not accessible in the if: condition [0], so set an output variable [1] - # [0] https://github.community/t/16928 - # [1] https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-output-parameter - - name: Check for PyPI token on tag - id: deployable - if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags') - env: - PYPI_API_TOKEN: "${{ secrets.PYPI_API_TOKEN }}" - run: if [ -n "$PYPI_API_TOKEN" ]; then echo ::set-output name=DEPLOY::true; fi - - - name: Upload to PyPI - if: steps.deployable.outputs.DEPLOY - uses: pypa/gh-action-pypi-publish@release/v1 - with: - user: __token__ - password: ${{ secrets.PYPI_API_TOKEN }} - packages-dir: interfaces/pydra/src/dist/ From d71093d07fb4a7b81fe8ccbd19cc28b28a57e4a6 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Thu, 21 Aug 2025 15:00:47 +1000 Subject: [PATCH 03/27] added in missing imports to app.py --- python/mrtrix3/app.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/python/mrtrix3/app.py b/python/mrtrix3/app.py index d41eba8740..2994d4e998 100644 --- a/python/mrtrix3/app.py +++ b/python/mrtrix3/app.py @@ -16,7 +16,15 @@ import argparse, importlib, inspect, math, os, pathlib, random, shlex, shutil, signal, string, subprocess, sys, textwrap, time from mrtrix3 import ANSI, CONFIG, MRtrixError, setup_ansi from mrtrix3 import utils, version - +import re +import typing as ty +from keyword import kwlist as PYTHON_KEYWORDS +try: + import black.parsing +except ImportError: + HAVE_BLACK = False +else: + HAVE_BLACK = True # These global constants can / should be accessed directly by scripts: From 4a71da2bdda76b2f4c7abdb88e105e2be1265828 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Thu, 21 Aug 2025 15:02:30 +1000 Subject: [PATCH 04/27] added in SequenceDirectoryOut Python type --- python/mrtrix3/app.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/python/mrtrix3/app.py b/python/mrtrix3/app.py index 2994d4e998..ae89a3d48a 100644 --- a/python/mrtrix3/app.py +++ b/python/mrtrix3/app.py @@ -745,6 +745,20 @@ def _legacytypestring(): @staticmethod def _metavar(): return 'values' + + + # Would you mind if this is defined here instead of in commands.population_template.usage + # makes it much easiery to import and include in instance checks for pydra auto-gen?? + class SequenceDirectoryOut(CustomTypeBase): + def __call__(self, input_value): + return [Parser.make_userpath_object(Parser._UserDirOutPathExtras, item) # pylint: disable=protected-access \ + for item in input_value.split(',')] + @staticmethod + def _legacytypestring(): + return 'SEQDIROUT' + @staticmethod + def _metavar(): + return 'directory_list' class DirectoryIn(CustomTypeBase): def __call__(self, input_value): From a6a10972e61887a8881a0a868237086f50ca4e6d Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Thu, 21 Aug 2025 15:03:27 +1000 Subject: [PATCH 05/27] touch up --- python/mrtrix3/app.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python/mrtrix3/app.py b/python/mrtrix3/app.py index ae89a3d48a..1f67975447 100644 --- a/python/mrtrix3/app.py +++ b/python/mrtrix3/app.py @@ -27,6 +27,7 @@ HAVE_BLACK = True + # These global constants can / should be accessed directly by scripts: # - 'ARGS' will contain the user's command-line inputs upon parsing of the command-line # - 'CONTINUE_OPTION' will be set to True if the user provides the -continue option; From ce7920ad9ef4325348578e68ab8e558e570570a1 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Fri, 22 Aug 2025 09:44:54 +1000 Subject: [PATCH 06/27] debugged pydra code generation --- cpp/core/app.cpp | 974 ++++++++++++++++++++---------------------- python/mrtrix3/app.py | 3 + 2 files changed, 470 insertions(+), 507 deletions(-) diff --git a/cpp/core/app.cpp b/cpp/core/app.cpp index 12e78e9916..59231eef40 100644 --- a/cpp/core/app.cpp +++ b/cpp/core/app.cpp @@ -1247,511 +1247,435 @@ std::string pydra_usage() { return s; } -const Option *match_option(const char *arg) { - if (consume_dash(arg) && *arg && !isdigit(*arg) && *arg != '.') { - while (consume_dash(arg)) - ; - std::vector candidates; - std::string root(arg); - - for (size_t i = 0; i < OPTIONS.size(); ++i) - get_matches(candidates, OPTIONS[i], root); - get_matches(candidates, __standard_options, root); - - // no matches - if (candidates.size() == 0) - throw Exception(std::string("unknown option \"-") + root + "\""); - - // return match if unique: - if (candidates.size() == 1) - return candidates[0]; - - // return match if fully specified: - for (size_t i = 0; i < candidates.size(); ++i) - if (root == candidates[i]->id) - return candidates[i]; - - // check if there is only one *unique* candidate - const auto cid = candidates[0]->id; - if (std::all_of(++candidates.begin(), candidates.end(), [&cid](const Option *cand) { return cand->id == cid; })) - return candidates[0]; - - // report something useful: - root = "several matches possible for option \"-" + root + "\": \"-" + candidates[0]->id; - - for (size_t i = 1; i < candidates.size(); ++i) - root += std::string("\", \"-") + candidates[i]->id + "\""; - - throw Exception(root); +const Option *match_option(std::string_view arg) { + auto no_dash_arg = without_leading_dash(arg); + if (arg.size() == no_dash_arg.size() || no_dash_arg.empty() || isdigit(no_dash_arg.front()) != 0 || + no_dash_arg.front() == '.') { + return nullptr; } - const Option *match_option(std::string_view arg) { - auto no_dash_arg = without_leading_dash(arg); - if (arg.size() == no_dash_arg.size() || no_dash_arg.empty() || isdigit(no_dash_arg.front()) != 0 || - no_dash_arg.front() == '.') { - return nullptr; - } - - std::vector candidates; - std::string root(no_dash_arg); - - for (size_t i = 0; i < OPTIONS.size(); ++i) - get_matches(candidates, OPTIONS[i], root); - get_matches(candidates, __standard_options, root); + std::vector candidates; + std::string root(no_dash_arg); - // no matches - if (candidates.empty()) - throw Exception(std::string("unknown option \"-") + root + "\""); + for (size_t i = 0; i < OPTIONS.size(); ++i) + get_matches(candidates, OPTIONS[i], root); + get_matches(candidates, __standard_options, root); - // return match if unique: - if (candidates.size() == 1) - return candidates[0]; + // no matches + if (candidates.empty()) + throw Exception(std::string("unknown option \"-") + root + "\""); - // return match if fully specified: - for (size_t i = 0; i < candidates.size(); ++i) - if (root == candidates[i]->id) - return candidates[i]; + // return match if unique: + if (candidates.size() == 1) + return candidates[0]; - // check if there is only one *unique* candidate - const auto cid = candidates[0]->id; - if (std::all_of(++candidates.begin(), candidates.end(), [&cid](const Option *cand) { return cand->id == cid; })) - return candidates[0]; + // return match if fully specified: + for (size_t i = 0; i < candidates.size(); ++i) + if (root == candidates[i]->id) + return candidates[i]; - // report something useful: - root = "several matches possible for option \"-" + root + "\": \"-" + candidates[0]->id; + // check if there is only one *unique* candidate + const auto cid = candidates[0]->id; + if (std::all_of(++candidates.begin(), candidates.end(), [&cid](const Option *cand) { return cand->id == cid; })) + return candidates[0]; - for (size_t i = 1; i < candidates.size(); ++i) - root += std::string("\", \"-") + candidates[i]->id + "\""; + // report something useful: + root = "several matches possible for option \"-" + root + "\": \"-" + candidates[0]->id; - throw Exception(root); - } + for (size_t i = 1; i < candidates.size(); ++i) + root += std::string("\", \"-") + candidates[i]->id + "\""; - void sort_arguments(const std::vector &arguments) { - auto it = arguments.begin(); - while (it != arguments.end()) { - const size_t index = std::distance(arguments.begin(), it); - const Option *opt = match_option(*it); - if (opt != nullptr) { - if (it + opt->size() >= arguments.end()) { - throw Exception(std::string("not enough parameters to option \"-") + opt->id + "\""); - } + throw Exception(root); +} - std::vector option_args; - std::copy_n(it + 1, opt->size(), std::back_inserter(option_args)); - option.push_back(ParsedOption(opt, option_args, index)); - it += opt->size(); - } else { - argument.push_back(ParsedArgument(nullptr, nullptr, *it, index)); +void sort_arguments(const std::vector &arguments) { + auto it = arguments.begin(); + while (it != arguments.end()) { + const size_t index = std::distance(arguments.begin(), it); + const Option *opt = match_option(*it); + if (opt != nullptr) { + if (it + opt->size() >= arguments.end()) { + throw Exception(std::string("not enough parameters to option \"-") + opt->id + "\""); } - ++it; - } - } - void parse_standard_options() { - if (!get_options("info").empty()) - log_level = std::max(log_level, 2); - if (!get_options("debug").empty()) - log_level = 3; - if (!get_options("quiet").empty()) - log_level = 0; - if (!get_options("force").empty()) { - WARN("existing output files will be overwritten"); - overwrite_files = true; + std::vector option_args; + std::copy_n(it + 1, opt->size(), std::back_inserter(option_args)); + option.push_back(ParsedOption(opt, option_args, index)); + it += opt->size(); + } else { + argument.push_back(ParsedArgument(nullptr, nullptr, *it, index)); } + ++it; } +} - void verify_usage() { - if (AUTHOR.empty()) - throw Exception("No author specified for command " + std::string(NAME)); - if (SYNOPSIS.empty()) - throw Exception("No synopsis specified for command " + std::string(NAME)); +void parse_standard_options() { + if (!get_options("info").empty()) + log_level = std::max(log_level, 2); + if (!get_options("debug").empty()) + log_level = 3; + if (!get_options("quiet").empty()) + log_level = 0; + if (!get_options("force").empty()) { + WARN("existing output files will be overwritten"); + overwrite_files = true; } +} - void parse_special_options() { - // special options are only accessible as the first argument - if (raw_arguments_list.size() != 1) - return; +void verify_usage() { + if (AUTHOR.empty()) + throw Exception("No author specified for command " + std::string(NAME)); + if (SYNOPSIS.empty()) + throw Exception("No synopsis specified for command " + std::string(NAME)); +} - if (raw_arguments_list.front() == "__print_full_usage__") { - print(full_usage()); - throw 0; - } - if (raw_arguments_list.front() == "__print_usage_markdown__") { - print(markdown_usage()); - throw 0; - } - if (raw_arguments_list.front() == "__print_usage_rst__") { - print(restructured_text_usage()); - throw 0; - } - if (raw_arguments_list.front() == "__print_usage_pydra__") { - print(pydra_usage()); - throw 0; - } - if (raw_arguments_list.front() == "__print_synopsis__") { - print(SYNOPSIS); - throw 0; - } +void parse_special_options() { + // special options are only accessible as the first argument + if (raw_arguments_list.size() != 1) + return; + + if (raw_arguments_list.front() == "__print_full_usage__") { + print(full_usage()); + throw 0; + } + if (raw_arguments_list.front() == "__print_usage_markdown__") { + print(markdown_usage()); + throw 0; + } + if (raw_arguments_list.front() == "__print_usage_rst__") { + print(restructured_text_usage()); + throw 0; + } + if (raw_arguments_list.front() == "__print_usage_pydra__") { + print(pydra_usage()); + throw 0; + } + if (raw_arguments_list.front() == "__print_synopsis__") { + print(SYNOPSIS); + throw 0; } +} - void parse() { - argument.clear(); - option.clear(); +void parse() { + argument.clear(); + option.clear(); - sort_arguments(raw_arguments_list); + sort_arguments(raw_arguments_list); - if (!get_options("help").empty()) { - print_help(); - throw 0; - } - if (!get_options("version").empty()) { - print(version_string()); - throw 0; - } + if (!get_options("help").empty()) { + print_help(); + throw 0; + } + if (!get_options("version").empty()) { + print(version_string()); + throw 0; + } + + size_t num_args_required = 0; + size_t num_optional_arguments = 0; - size_t num_args_required = 0; - size_t num_optional_arguments = 0; - - ArgFlags flags = None; - for (size_t i = 0; i < ARGUMENTS.size(); ++i) { - if (ARGUMENTS[i].flags) { - if (flags && flags != ARGUMENTS[i].flags) - throw Exception("FIXME: all arguments declared optional() or allow_multiple() should have matching flags in " - "command-line syntax"); - flags = ARGUMENTS[i].flags; - ++num_optional_arguments; - if (!(flags & Optional)) - ++num_args_required; - } else + ArgFlags flags = None; + for (size_t i = 0; i < ARGUMENTS.size(); ++i) { + if (ARGUMENTS[i].flags) { + if (flags && flags != ARGUMENTS[i].flags) + throw Exception("FIXME: all arguments declared optional() or allow_multiple() should have matching flags in " + "command-line syntax"); + flags = ARGUMENTS[i].flags; + ++num_optional_arguments; + if (!(flags & Optional)) ++num_args_required; - } + } else + ++num_args_required; + } - if (option.empty() && argument.empty() && REQUIRES_AT_LEAST_ONE_ARGUMENT) { - print_help(); - throw 0; - } + if (option.empty() && argument.empty() && REQUIRES_AT_LEAST_ONE_ARGUMENT) { + print_help(); + throw 0; + } - if (num_optional_arguments && num_args_required > argument.size()) - throw Exception("Expected at least " + str(num_args_required) + " arguments (" + str(argument.size()) + - " supplied)"); - - if (num_optional_arguments == 0 && num_args_required != argument.size()) { - Exception e("Expected exactly " + str(num_args_required) + " arguments (" + str(argument.size()) + " supplied)"); - std::string s = "Usage: " + NAME; - for (const auto &a : ARGUMENTS) - s += " " + std::string(a.id); - e.push_back(s); - s = "Yours: " + NAME; - for (const auto &a : argument) - s += " " + std::string(a); - e.push_back(s); - if (argument.size() > num_args_required) { - std::vector potential_options; - for (const auto &a : argument) { - for (const auto &og : OPTIONS) { - for (const auto &o : og) { - if (std::string(a) == std::string(o.id)) - potential_options.push_back("'-" + a + "'"); - } + if (num_optional_arguments && num_args_required > argument.size()) + throw Exception("Expected at least " + str(num_args_required) + " arguments (" + str(argument.size()) + + " supplied)"); + + if (num_optional_arguments == 0 && num_args_required != argument.size()) { + Exception e("Expected exactly " + str(num_args_required) + " arguments (" + str(argument.size()) + " supplied)"); + std::string s = "Usage: " + NAME; + for (const auto &a : ARGUMENTS) + s += " " + std::string(a.id); + e.push_back(s); + s = "Yours: " + NAME; + for (const auto &a : argument) + s += " " + std::string(a); + e.push_back(s); + if (argument.size() > num_args_required) { + std::vector potential_options; + for (const auto &a : argument) { + for (const auto &og : OPTIONS) { + for (const auto &o : og) { + if (std::string(a) == std::string(o.id)) + potential_options.push_back("'-" + a + "'"); } } - if (!potential_options.empty()) - e.push_back("(Did you mean " + join(potential_options, " or ") + "?)"); } - throw e; + if (!potential_options.empty()) + e.push_back("(Did you mean " + join(potential_options, " or ") + "?)"); } + throw e; + } - size_t num_extra_arguments = argument.size() - num_args_required; - size_t num_arg_per_multi = num_optional_arguments ? num_extra_arguments / num_optional_arguments : 0; - if (num_arg_per_multi * num_optional_arguments != num_extra_arguments) - throw Exception("number of optional arguments provided are not equal for all arguments"); - if (!(flags & Optional)) - ++num_arg_per_multi; - - // assign arguments to their corresponding definitions: - for (size_t n = 0, index = 0, next = 0; n < argument.size(); ++n) { - if (n == next) { - if (n) - ++index; - if (ARGUMENTS[index].flags != None) - next = n + num_arg_per_multi; - else - ++next; - } - argument[n].arg = &ARGUMENTS[index]; + size_t num_extra_arguments = argument.size() - num_args_required; + size_t num_arg_per_multi = num_optional_arguments ? num_extra_arguments / num_optional_arguments : 0; + if (num_arg_per_multi * num_optional_arguments != num_extra_arguments) + throw Exception("number of optional arguments provided are not equal for all arguments"); + if (!(flags & Optional)) + ++num_arg_per_multi; + + // assign arguments to their corresponding definitions: + for (size_t n = 0, index = 0, next = 0; n < argument.size(); ++n) { + if (n == next) { + if (n) + ++index; + if (ARGUMENTS[index].flags != None) + next = n + num_arg_per_multi; + else + ++next; } + argument[n].arg = &ARGUMENTS[index]; + } - // check for multiple instances of options: - for (size_t i = 0; i < OPTIONS.size(); ++i) { - for (size_t j = 0; j < OPTIONS[i].size(); ++j) { - size_t count = 0; - for (size_t k = 0; k < option.size(); ++k) - if (option[k].opt == &OPTIONS[i][j]) - count++; + // check for multiple instances of options: + for (size_t i = 0; i < OPTIONS.size(); ++i) { + for (size_t j = 0; j < OPTIONS[i].size(); ++j) { + size_t count = 0; + for (size_t k = 0; k < option.size(); ++k) + if (option[k].opt == &OPTIONS[i][j]) + count++; - if (count < 1 && !(OPTIONS[i][j].flags & Optional)) - throw Exception(std::string("mandatory option \"-") + OPTIONS[i][j].id + "\" must be specified"); + if (count < 1 && !(OPTIONS[i][j].flags & Optional)) + throw Exception(std::string("mandatory option \"-") + OPTIONS[i][j].id + "\" must be specified"); - if (count > 1 && !(OPTIONS[i][j].flags & AllowMultiple)) - throw Exception(std::string("multiple instances of option \"-") + OPTIONS[i][j].id + "\" are not allowed"); - } + if (count > 1 && !(OPTIONS[i][j].flags & AllowMultiple)) + throw Exception(std::string("multiple instances of option \"-") + OPTIONS[i][j].id + "\" are not allowed"); } + } - parse_standard_options(); - - File::Config::init(); - - // CONF option: FailOnWarn - // CONF default: 0 (false) - // CONF A boolean value specifying whether MRtrix applications should - // CONF abort as soon as any (otherwise non-fatal) warning is issued. - fail_on_warn = File::Config::get_bool("FailOnWarn", false); + parse_standard_options(); - // CONF option: TerminalColor - // CONF default: 1 (true) - // CONF A boolean value to indicate whether colours should be used in the terminal. - terminal_use_colour = File::Config::get_bool("TerminalColor", terminal_use_colour); + File::Config::init(); - // check for the existence of all specified input files (including optional ones that have been provided) - // if necessary, also check for pre-existence of any output files with known paths - // (if the output is e.g. given as a prefix, the argument should be flagged as type_text()) - for (const auto &i : argument) { - const std::string text = std::string(i); - if (i.arg->type == ArgFileIn || i.arg->type == TracksIn) { + // CONF option: FailOnWarn + // CONF default: 0 (false) + // CONF A boolean value specifying whether MRtrix applications should + // CONF abort as soon as any (otherwise non-fatal) warning is issued. + fail_on_warn = File::Config::get_bool("FailOnWarn", false); + + // CONF option: TerminalColor + // CONF default: 1 (true) + // CONF A boolean value to indicate whether colours should be used in the terminal. + terminal_use_colour = File::Config::get_bool("TerminalColor", terminal_use_colour); + + // check for the existence of all specified input files (including optional ones that have been provided) + // if necessary, also check for pre-existence of any output files with known paths + // (if the output is e.g. given as a prefix, the argument should be flagged as type_text()) + for (const auto &i : argument) { + const std::string text = std::string(i); + if (i.arg->type == ArgFileIn || i.arg->type == TracksIn) { + if (!Path::exists(text)) + throw Exception("required input file \"" + text + "\" not found"); + if (!Path::is_file(text)) + throw Exception("required input \"" + text + "\" is not a file"); + } + if (i.arg->type == ArgDirectoryIn) { + if (!Path::exists(text)) + throw Exception("required input directory \"" + text + "\" not found"); + if (!Path::is_dir(text)) + throw Exception("required input \"" + text + "\" is not a directory"); + } + if (i.arg->type == ArgFileOut || i.arg->type == TracksOut) { + if (text.find_last_of(PATH_SEPARATORS) == text.size() - 1) + throw Exception("output path \"" + std::string(i) + + "\" is not a valid file path (ends with directory path separator)"); + check_overwrite(text); + } + if (i.arg->type == ArgDirectoryOut) + check_overwrite(text); + if (i.arg->type == TracksIn && !Path::has_suffix(text, ".tck")) + throw Exception("input file \"" + text + "\" is not a valid track file"); + if (i.arg->type == TracksOut && !Path::has_suffix(text, ".tck")) + throw Exception("output track file \"" + text + "\" must use the .tck suffix"); + } + for (const auto &i : option) { + for (size_t j = 0; j != i.opt->size(); ++j) { + const Argument &arg = i.opt->operator[](j); + const std::string text = std::string(i.args[j]); + if (arg.type == ArgFileIn || arg.type == TracksIn) { if (!Path::exists(text)) - throw Exception("required input file \"" + text + "\" not found"); + throw Exception("input file \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" not found"); if (!Path::is_file(text)) - throw Exception("required input \"" + text + "\" is not a file"); + throw Exception("input \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" is not a file"); } - if (i.arg->type == ArgDirectoryIn) { + if (arg.type == ArgDirectoryIn) { if (!Path::exists(text)) - throw Exception("required input directory \"" + text + "\" not found"); + throw Exception("input directory \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" not found"); if (!Path::is_dir(text)) - throw Exception("required input \"" + text + "\" is not a directory"); + throw Exception("input \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" is not a directory"); } - if (i.arg->type == ArgFileOut || i.arg->type == TracksOut) { + if (arg.type == ArgFileOut || arg.type == TracksOut) { if (text.find_last_of(PATH_SEPARATORS) == text.size() - 1) - throw Exception("output path \"" + std::string(i) + + throw Exception("output path \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" is not a valid file path (ends with directory path separator)"); check_overwrite(text); } - if (i.arg->type == ArgDirectoryOut) + if (arg.type == ArgDirectoryOut) check_overwrite(text); - if (i.arg->type == TracksIn && !Path::has_suffix(text, ".tck")) - throw Exception("input file \"" + text + "\" is not a valid track file"); - if (i.arg->type == TracksOut && !Path::has_suffix(text, ".tck")) - throw Exception("output track file \"" + text + "\" must use the .tck suffix"); + if (arg.type == TracksIn && !Path::has_suffix(text, ".tck")) + throw Exception("input file \"" + text + "\" for option \"-" + std::string(i.opt->id) + + "\" is not a valid track file"); + if (arg.type == TracksOut && !Path::has_suffix(text, ".tck")) + throw Exception("output track file \"" + text + "\" for option \"-" + std::string(i.opt->id) + + "\" must use the .tck suffix"); } - for (const auto &i : option) { - for (size_t j = 0; j != i.opt->size(); ++j) { - const Argument &arg = i.opt->operator[](j); - const std::string text = std::string(i.args[j]); - if (arg.type == ArgFileIn || arg.type == TracksIn) { - if (!Path::exists(text)) - throw Exception("input file \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" not found"); - if (!Path::is_file(text)) - throw Exception("input \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" is not a file"); - } - if (arg.type == ArgDirectoryIn) { - if (!Path::exists(text)) - throw Exception("input directory \"" + text + "\" for option \"-" + std::string(i.opt->id) + - "\" not found"); - if (!Path::is_dir(text)) - throw Exception("input \"" + text + "\" for option \"-" + std::string(i.opt->id) + "\" is not a directory"); - } - if (arg.type == ArgFileOut || arg.type == TracksOut) { - if (text.find_last_of(PATH_SEPARATORS) == text.size() - 1) - throw Exception("output path \"" + text + "\" for option \"-" + std::string(i.opt->id) + - "\" is not a valid file path (ends with directory path separator)"); - check_overwrite(text); - } - if (arg.type == ArgDirectoryOut) - check_overwrite(text); - if (arg.type == TracksIn && !Path::has_suffix(text, ".tck")) - throw Exception("input file \"" + text + "\" for option \"-" + std::string(i.opt->id) + - "\" is not a valid track file"); - if (arg.type == TracksOut && !Path::has_suffix(text, ".tck")) - throw Exception("output track file \"" + text + "\" for option \"-" + std::string(i.opt->id) + - "\" must use the .tck suffix"); - } - } - - SignalHandler::init(); } - void init(int cmdline_argc, const char *const *cmdline_argv) { + SignalHandler::init(); +} + +void init(int cmdline_argc, const char *const *cmdline_argv) { #ifdef MRTRIX_WINDOWS - // force stderr to be unbuffered, and stdout to be line-buffered: - setvbuf(stderr, nullptr, _IONBF, 0); - setvbuf(stdout, nullptr, _IOLBF, 0); + // force stderr to be unbuffered, and stdout to be line-buffered: + setvbuf(stderr, nullptr, _IONBF, 0); + setvbuf(stdout, nullptr, _IOLBF, 0); #endif - terminal_use_colour = !ProgressBar::set_update_method(); + terminal_use_colour = !ProgressBar::set_update_method(); - raw_arguments_list = std::vector(cmdline_argv, cmdline_argv + cmdline_argc); - NAME = Path::basename(raw_arguments_list.front()); - raw_arguments_list.erase(raw_arguments_list.begin()); + raw_arguments_list = std::vector(cmdline_argv, cmdline_argv + cmdline_argc); + NAME = Path::basename(raw_arguments_list.front()); + raw_arguments_list.erase(raw_arguments_list.begin()); #ifdef MRTRIX_WINDOWS - if (Path::has_suffix(NAME, ".exe")) - NAME.erase(NAME.size() - 4); + if (Path::has_suffix(NAME, ".exe")) + NAME.erase(NAME.size() - 4); #endif - auto argv_quoted = [](const std::string &s) -> std::string { - for (size_t i = 0; i != s.size(); ++i) { - if (!(isalnum(s[i]) || s[i] == '.' || s[i] == '_' || s[i] == '-' || s[i] == '/')) { - std::string escaped_string("\'"); - for (auto c : s) { - switch (c) { - case '\'': - escaped_string.append("\\\'"); - break; - case '\\': - escaped_string.append("\\\\"); - break; - default: - escaped_string.push_back(c); - break; - } + auto argv_quoted = [](const std::string &s) -> std::string { + for (size_t i = 0; i != s.size(); ++i) { + if (!(isalnum(s[i]) || s[i] == '.' || s[i] == '_' || s[i] == '-' || s[i] == '/')) { + std::string escaped_string("\'"); + for (auto c : s) { + switch (c) { + case '\'': + escaped_string.append("\\\'"); + break; + case '\\': + escaped_string.append("\\\\"); + break; + default: + escaped_string.push_back(c); + break; } - escaped_string.push_back('\''); - return escaped_string; } + escaped_string.push_back('\''); + return escaped_string; } - return s; - }; - command_history_string = cmdline_argv[0]; - for (const auto &a : raw_arguments_list) - command_history_string += std::string(" ") + argv_quoted(a); - command_history_string += std::string(" (version=") + mrtrix_version; - if (project_version) - command_history_string += std::string(", project=") + project_version; - command_history_string += ")"; - - std::locale::global(std::locale::classic()); - std::setlocale(LC_ALL, "C"); - - srand(time(nullptr)); - } - - const std::vector get_options(const std::string &name) { - assert(!name.empty()); - assert(name[0] != '-'); - std::vector matches; - for (size_t i = 0; i < option.size(); ++i) { - assert(option[i].opt); - if (option[i].opt->is(name)) - matches.push_back({option[i].opt, option[i].args, option[i].index}); } - return matches; + return s; + }; + command_history_string = cmdline_argv[0]; + for (const auto &a : raw_arguments_list) + command_history_string += std::string(" ") + argv_quoted(a); + command_history_string += std::string(" (version=") + mrtrix_version; + if (project_version) + command_history_string += std::string(", project=") + project_version; + command_history_string += ")"; + + std::locale::global(std::locale::classic()); + std::setlocale(LC_ALL, "C"); + + srand(time(nullptr)); +} + +const std::vector get_options(const std::string &name) { + assert(!name.empty()); + assert(name[0] != '-'); + std::vector matches; + for (size_t i = 0; i < option.size(); ++i) { + assert(option[i].opt); + if (option[i].opt->is(name)) + matches.push_back({option[i].opt, option[i].args, option[i].index}); } + return matches; +} - int64_t App::ParsedArgument::as_int() const { - if (arg->type == Integer) { - - // Check to see if there are any alpha characters in here - // - If a single character at the end, use as integer multiplier - // - Unless there's a dot point before the multiplier; in which case, - // parse the number as a float, multiply, then cast to integer - // - If a single 'e' or 'E' in the middle, parse as float and convert to integer - size_t alpha_count = 0; - bool alpha_is_last = false; - bool contains_dotpoint = false; - char alpha_char = 0; - for (const char &c : p) { - if (std::isalpha(c) != 0) { - ++alpha_count; - alpha_is_last = true; - alpha_char = c; - } else { - alpha_is_last = false; - } - if (c == '.') - contains_dotpoint = true; +int64_t App::ParsedArgument::as_int() const { + if (arg->type == Integer) { + + // Check to see if there are any alpha characters in here + // - If a single character at the end, use as integer multiplier + // - Unless there's a dot point before the multiplier; in which case, + // parse the number as a float, multiply, then cast to integer + // - If a single 'e' or 'E' in the middle, parse as float and convert to integer + size_t alpha_count = 0; + bool alpha_is_last = false; + bool contains_dotpoint = false; + char alpha_char = 0; + for (const char &c : p) { + if (std::isalpha(c) != 0) { + ++alpha_count; + alpha_is_last = true; + alpha_char = c; + } else { + alpha_is_last = false; } - if (alpha_count > 1) - throw Exception("error converting string " + str(p) + " to integer: too many letters"); - int64_t retval = 0; - if (alpha_count) { - if (alpha_is_last) { - std::string num(p); - const char postfix = num.back(); - num.pop_back(); - int64_t multiplier = 1.0; - switch (postfix) { - case 'k': - case 'K': - multiplier = 1000; - break; - case 'm': - case 'M': - multiplier = 1000000; - break; - case 'b': - case 'B': - multiplier = 1000000000; - break; - case 't': - case 'T': - multiplier = 1000000000000; - break; - default: - throw Exception("error converting string " + str(p) + " to integer: unexpected postfix \'" + postfix + - "\'"); - } - if (contains_dotpoint) { - const default_type prefix = to(num); - retval = std::round(prefix * default_type(multiplier)); - } else { - retval = to(num) * multiplier; - } - } else if (alpha_char == 'e' || alpha_char == 'E') { - const default_type as_float = to(p); - retval = std::round(as_float); + if (c == '.') + contains_dotpoint = true; + } + if (alpha_count > 1) + throw Exception("error converting string " + str(p) + " to integer: too many letters"); + int64_t retval = 0; + if (alpha_count) { + if (alpha_is_last) { + std::string num(p); + const char postfix = num.back(); + num.pop_back(); + int64_t multiplier = 1.0; + switch (postfix) { + case 'k': + case 'K': + multiplier = 1000; + break; + case 'm': + case 'M': + multiplier = 1000000; + break; + case 'b': + case 'B': + multiplier = 1000000000; + break; + case 't': + case 'T': + multiplier = 1000000000000; + break; + default: + throw Exception("error converting string " + str(p) + " to integer: unexpected postfix \'" + postfix + "\'"); + } + if (contains_dotpoint) { + const default_type prefix = to(num); + retval = std::round(prefix * default_type(multiplier)); } else { - throw Exception("error converting string " + str(p) + " to integer: unexpected character"); + retval = to(num) * multiplier; } + } else if (alpha_char == 'e' || alpha_char == 'E') { + const default_type as_float = to(p); + retval = std::round(as_float); } else { - retval = to(p); - } - - const auto range = std::get(arg->limits); - if (retval < range.min || retval > range.max) { - std::string msg("value supplied for "); - if (opt) - msg += std::string("option \"") + opt->id; - else - msg += std::string("argument \"") + arg->id; - msg += "\" is out of bounds (valid range: " + str(range.min) + " to " + str(range.max) + - ", value supplied: " + str(retval) + ")"; - throw Exception(msg); - } - return retval; - } - - if (arg->type == Choice) { - std::string selection = lowercase(p); - const auto &choices = std::get>(arg->limits); - auto it = std::find(choices.begin(), choices.end(), selection); - if (it == choices.end()) { - std::string msg = std::string("unexpected value supplied for "); - if (opt != nullptr) - msg += std::string("option \"") + opt->id; - else - msg += std::string("argument \"") + arg->id; - msg += std::string("\" (received \"" + std::string(p) + "\"; valid choices are: ") + join(choices, ", ") + ")"; - throw Exception(msg); + throw Exception("error converting string " + str(p) + " to integer: unexpected character"); } - return static_cast(std::distance(choices.begin(), it)); + } else { + retval = to(p); } - assert(0); - return (0); - } - default_type App::ParsedArgument::as_float() const { - assert(arg->type == Float); - const default_type retval = to(p); - const auto range = std::get(arg->limits); + const auto range = std::get(arg->limits); if (retval < range.min || retval > range.max) { std::string msg("value supplied for "); if (opt) @@ -1762,103 +1686,139 @@ const Option *match_option(const char *arg) { ", value supplied: " + str(retval) + ")"; throw Exception(msg); } - return retval; } - std::vector ParsedArgument::as_sequence_int() const { - assert(arg->type == IntSeq); - try { - return parse_ints(p); - } catch (Exception &e) { - error(e); + if (arg->type == Choice) { + std::string selection = lowercase(p); + const auto &choices = std::get>(arg->limits); + auto it = std::find(choices.begin(), choices.end(), selection); + if (it == choices.end()) { + std::string msg = std::string("unexpected value supplied for "); + if (opt != nullptr) + msg += std::string("option \"") + opt->id; + else + msg += std::string("argument \"") + arg->id; + msg += std::string("\" (received \"" + std::string(p) + "\"; valid choices are: ") + join(choices, ", ") + ")"; + throw Exception(msg); } - return std::vector(); + return static_cast(std::distance(choices.begin(), it)); } + assert(0); + return (0); +} - std::vector ParsedArgument::as_sequence_uint() const { - assert(arg->type == IntSeq); - try { - return parse_ints(p); - } catch (Exception &e) { - error(e); - } - return std::vector(); +default_type App::ParsedArgument::as_float() const { + assert(arg->type == Float); + const default_type retval = to(p); + const auto range = std::get(arg->limits); + if (retval < range.min || retval > range.max) { + std::string msg("value supplied for "); + if (opt) + msg += std::string("option \"") + opt->id; + else + msg += std::string("argument \"") + arg->id; + msg += "\" is out of bounds (valid range: " + str(range.min) + " to " + str(range.max) + + ", value supplied: " + str(retval) + ")"; + throw Exception(msg); } - std::vector ParsedArgument::as_sequence_float() const { - assert(arg->type == FloatSeq); - try { - return parse_floats(p); - } catch (Exception &e) { - error(e); - } - return std::vector(); - } + return retval; +} - ParsedArgument::ParsedArgument(const Option *option, const Argument *argument, std::string text, size_t index) - : opt(option), arg(argument), p(std::move(text)), index_(index) { - assert(!p.empty()); +std::vector ParsedArgument::as_sequence_int() const { + assert(arg->type == IntSeq); + try { + return parse_ints(p); + } catch (Exception &e) { + error(e); } + return std::vector(); +} - void ParsedArgument::error(Exception & e) const { - std::string msg("error parsing token \""); - msg += p; - if (opt != nullptr) - msg += std::string("\" for option \"") + opt->id + "\""; - else - msg += std::string("\" for argument \"") + arg->id + "\""; - throw Exception(e, msg); +std::vector ParsedArgument::as_sequence_uint() const { + assert(arg->type == IntSeq); + try { + return parse_ints(p); + } catch (Exception &e) { + error(e); } + return std::vector(); +} - void check_overwrite(const std::string &name) { - if (Path::exists(name) && !overwrite_files) { - if (check_overwrite_files_func != nullptr) - check_overwrite_files_func(name); - else - throw Exception("output path \"" + name + "\" already exists (use -force option to force overwrite)"); - } +std::vector ParsedArgument::as_sequence_float() const { + assert(arg->type == FloatSeq); + try { + return parse_floats(p); + } catch (Exception &e) { + error(e); } + return std::vector(); +} - ParsedOption::ParsedOption(const Option *option, const std::vector &arguments, size_t i) - : opt(option), args(arguments), index(i) { - for (size_t i = 0; i != option->size(); ++i) { - const auto &p = arguments[i]; - if (!starts_with_dash(p)) - continue; - if (((*option)[i].type == ImageIn || (*option)[i].type == ImageOut) && is_dash(arguments[i])) - continue; - if ((*option)[i].type == Integer || (*option)[i].type == Float || (*option)[i].type == IntSeq || - (*option)[i].type == FloatSeq || (*option)[i].type == Various) - continue; - WARN(std::string("Value \"") + arguments[i] + "\" is being used as " + - ((option->size() == 1) ? "the expected argument " - : ("one of the " + str(option->size()) + " expected arguments ")) + - "for option \"-" + option->id + "\", yet this itself looks like a separate command-line option; " + - "the requisite input" + ((option->size() == 1) ? " " : "s ") + "to command-line option \"-" + option->id + - "\" may have been erroneously omitted, which may cause " + "other command-line parsing errors"); - } - } +ParsedArgument::ParsedArgument(const Option *option, const Argument *argument, std::string text, size_t index) + : opt(option), arg(argument), p(std::move(text)), index_(index) { + assert(!p.empty()); +} - ParsedArgument ParsedOption::operator[](size_t num) const { - assert(num < opt->size()); - return ParsedArgument(opt, &(*opt)[num], args[num], index + num + 1); - } +void ParsedArgument::error(Exception &e) const { + std::string msg("error parsing token \""); + msg += p; + if (opt != nullptr) + msg += std::string("\" for option \"") + opt->id + "\""; + else + msg += std::string("\" for argument \"") + arg->id + "\""; + throw Exception(e, msg); +} - bool ParsedOption::operator==(const char *match) const { - const std::string name = lowercase(match); - return name == opt->id; +void check_overwrite(const std::string &name) { + if (Path::exists(name) && !overwrite_files) { + if (check_overwrite_files_func != nullptr) + check_overwrite_files_func(name); + else + throw Exception("output path \"" + name + "\" already exists (use -force option to force overwrite)"); } +} - std::string operator+(const char *left, const ParsedArgument &right) { - std::string retval(left); - retval += std::string(right); - return retval; +ParsedOption::ParsedOption(const Option *option, const std::vector &arguments, size_t i) + : opt(option), args(arguments), index(i) { + for (size_t i = 0; i != option->size(); ++i) { + const auto &p = arguments[i]; + if (!starts_with_dash(p)) + continue; + if (((*option)[i].type == ImageIn || (*option)[i].type == ImageOut) && is_dash(arguments[i])) + continue; + if ((*option)[i].type == Integer || (*option)[i].type == Float || (*option)[i].type == IntSeq || + (*option)[i].type == FloatSeq || (*option)[i].type == Various) + continue; + WARN(std::string("Value \"") + arguments[i] + "\" is being used as " + + ((option->size() == 1) ? "the expected argument " + : ("one of the " + str(option->size()) + " expected arguments ")) + + "for option \"-" + option->id + "\", yet this itself looks like a separate command-line option; " + + "the requisite input" + ((option->size() == 1) ? " " : "s ") + "to command-line option \"-" + option->id + + "\" may have been erroneously omitted, which may cause " + "other command-line parsing errors"); } +} - std::ostream &operator<<(std::ostream &stream, const ParsedArgument &arg) { - stream << std::string(arg); - return stream; - } +ParsedArgument ParsedOption::operator[](size_t num) const { + assert(num < opt->size()); + return ParsedArgument(opt, &(*opt)[num], args[num], index + num + 1); +} + +bool ParsedOption::operator==(const char *match) const { + const std::string name = lowercase(match); + return name == opt->id; +} + +std::string operator+(const char *left, const ParsedArgument &right) { + std::string retval(left); + retval += std::string(right); + return retval; +} + +std::ostream &operator<<(std::ostream &stream, const ParsedArgument &arg) { + stream << std::string(arg); + return stream; +} } // namespace MR::App diff --git a/python/mrtrix3/app.py b/python/mrtrix3/app.py index 1f67975447..4a3e56087d 100644 --- a/python/mrtrix3/app.py +++ b/python/mrtrix3/app.py @@ -156,6 +156,9 @@ def _execute(usage_function, execute_function): #pylint: disable=unused-variable elif sys.argv[-1] == '__print_usage_rst__': CMDLINE.print_usage_rst() sys.exit(0) + elif sys.argv[-1] == '__print_usage_pydra__': + CMDLINE.print_usage_pydra() + sys.exit(0) # Do the main command-line input parsing ARGS = CMDLINE.parse_args() From 3ebdad63bad1ec672e205826f877d5cc5ade1ea3 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Fri, 22 Aug 2025 11:22:24 +1000 Subject: [PATCH 07/27] renamed __print_usage_pydra__ to __print_pydra_code__ --- cpp/core/app.cpp | 5 ++--- python/mrtrix3/app.py | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cpp/core/app.cpp b/cpp/core/app.cpp index 59231eef40..e853690771 100644 --- a/cpp/core/app.cpp +++ b/cpp/core/app.cpp @@ -898,8 +898,7 @@ std::string pydra_usage() { "return", "True", "try", "while", "with", "yield", "container", "image", "container_xargs"}; // Add import lines - std::string s = - std::string("# Auto-generated from MRtrix C++ command with '__print_usage_pydra__' secret option\n\n"); + std::string s = std::string("# Auto-generated from MRtrix C++ command with '__print_pydra_code__' secret option\n\n"); s += "import typing as ty \n"; s += "from pathlib import Path # noqa: F401\n"; s += "from fileformats.generic import File, Directory # noqa: F401\n"; @@ -1346,7 +1345,7 @@ void parse_special_options() { print(restructured_text_usage()); throw 0; } - if (raw_arguments_list.front() == "__print_usage_pydra__") { + if (raw_arguments_list.front() == "__print_pydra_code__") { print(pydra_usage()); throw 0; } diff --git a/python/mrtrix3/app.py b/python/mrtrix3/app.py index 4a3e56087d..2d545293e9 100644 --- a/python/mrtrix3/app.py +++ b/python/mrtrix3/app.py @@ -156,7 +156,7 @@ def _execute(usage_function, execute_function): #pylint: disable=unused-variable elif sys.argv[-1] == '__print_usage_rst__': CMDLINE.print_usage_rst() sys.exit(0) - elif sys.argv[-1] == '__print_usage_pydra__': + elif sys.argv[-1] == '__print_pydra_code__': CMDLINE.print_usage_pydra() sys.exit(0) From 21ec26b6db12a9cb2772dfea3997023401d57bfe Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Fri, 22 Aug 2025 11:24:51 +1000 Subject: [PATCH 08/27] renamed generating methods to print_pydra_code from print_usage_pydra --- cpp/core/app.cpp | 4 ++-- python/mrtrix3/app.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cpp/core/app.cpp b/cpp/core/app.cpp index e853690771..16afd93412 100644 --- a/cpp/core/app.cpp +++ b/cpp/core/app.cpp @@ -847,7 +847,7 @@ std::string restructured_text_usage() { return s; } -std::string pydra_usage() { +std::string pydra_code() { std::string CMD_PREFIXES[] = {"Fivett", "Afd", "Amp", "Connectome", "Dcm", "Dir", "Dwi", "Fixel", "Fod", "Label", "Mask", "Mesh", "Mr", "Mt", "Peaks", "Sh", @@ -1346,7 +1346,7 @@ void parse_special_options() { throw 0; } if (raw_arguments_list.front() == "__print_pydra_code__") { - print(pydra_usage()); + print(pydra_code()); throw 0; } if (raw_arguments_list.front() == "__print_synopsis__") { diff --git a/python/mrtrix3/app.py b/python/mrtrix3/app.py index 2d545293e9..a968c4438d 100644 --- a/python/mrtrix3/app.py +++ b/python/mrtrix3/app.py @@ -157,7 +157,7 @@ def _execute(usage_function, execute_function): #pylint: disable=unused-variable CMDLINE.print_usage_rst() sys.exit(0) elif sys.argv[-1] == '__print_pydra_code__': - CMDLINE.print_usage_pydra() + CMDLINE.print_pydra_code() sys.exit(0) # Do the main command-line input parsing @@ -1507,14 +1507,14 @@ def print_group_options(group): alg, '__print_usage_rst__']) - def print_usage_pydra(self): + def print_pydra_code(self): if self._subparsers: if len(sys.argv) == 3: for alg in self._subparsers._group_actions[0].choices: if alg == sys.argv[-2]: - self._subparsers._group_actions[0].choices[alg].print_usage_pydra() + self._subparsers._group_actions[0].choices[alg].print_pydra_code() return self.error('Invalid subparser nominated: ' + sys.argv[-2]) assert len(sys.argv) == 2 @@ -1698,7 +1698,7 @@ def cmd_to_task_name(cmd_name: str) -> str: xor_str = "" text = ( - "# Auto-generated by mrtrix3/app.py:print_usage_pydra()\n\n" + "# Auto-generated by mrtrix3/app.py:print_pydra_code()\n\n" "import typing\n" "from pathlib import Path # noqa: F401\n" "from fileformats.generic import FsObject, File, Directory # noqa: F401\n" From 6e497b5f0012eee8f762ffb630c2f6e3a6d736f9 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Fri, 22 Aug 2025 15:07:13 +1000 Subject: [PATCH 09/27] dropped flake8 config --- .flake8 | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 .flake8 diff --git a/.flake8 b/.flake8 deleted file mode 100644 index 6a8f563a44..0000000000 --- a/.flake8 +++ /dev/null @@ -1,10 +0,0 @@ - -[flake8] -doctests = True -exclude = - **/__init__.py -max-line-length = 88 -select = C,E,F,W,B,B950 -extend-ignore = E203,E501,E129,E111,E303,E211,E201,E114,E261,E262,E226,E302,E121 -per-file-ignores = - setup.py:F401 From 4c48cddd85c1b15f9a7697d8f4788dc1f21fcb36 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Fri, 22 Aug 2025 15:09:18 +1000 Subject: [PATCH 10/27] reverted .gitignore --- .gitignore | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/.gitignore b/.gitignore index 9f38d7bd50..d9cf6aa55c 100644 --- a/.gitignore +++ b/.gitignore @@ -11,16 +11,8 @@ *.img *.hdr *.rste +/python/lib/mrtrix3/_version.py *.venv -config -build.* -/configure.log -/build.log -/core/version.cpp -/src/exec_version.cpp -/src/project_version.h -/lib/mrtrix3/_version.py -/test/src/project_version.h /scripts/mrtrix_bash_completion /dev/ /compiled_docs/ @@ -45,19 +37,3 @@ build/ CMakeLists.txt.user testing/data/tmp* testing/data/*-tmp-* -*.venv -/interfaces/pydra/src/pydra/tasks/mrtrix3/* -_version.py -version.cpp -exec_version.cpp -__pycache__ -.DS_Store - -# Pydra related ignores - -_version.py -/interfaces/pydra/src/pydra/tasks/mrtrix3/v3_0 -/interfaces/pydra/**/dist/** -.pytest_cache -/build* -/install From 49d4788540a8387a5eac3e69ffafddb4471c49a8 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Tue, 26 Aug 2025 16:45:24 +1000 Subject: [PATCH 11/27] renamed directions arguments to fibre_directions --- cpp/cmd/amp2response.cpp | 2 +- cpp/cmd/peaks2amp.cpp | 2 +- cpp/cmd/peaks2fixel.cpp | 2 +- cpp/cmd/sh2amp.cpp | 2 +- cpp/cmd/sh2response.cpp | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cpp/cmd/amp2response.cpp b/cpp/cmd/amp2response.cpp index ea8f3cdbb6..4a2c400b3a 100644 --- a/cpp/cmd/amp2response.cpp +++ b/cpp/cmd/amp2response.cpp @@ -57,7 +57,7 @@ void usage() { ARGUMENTS + Argument ("amps", "the amplitudes image").type_image_in() + Argument ("mask", "the mask containing the voxels from which to estimate the response function").type_image_in() - + Argument ("directions_image", "a 4D image containing the estimated fibre directions").type_image_in() + + Argument ("fibre_directions", "a 4D image containing the estimated fibre directions").type_image_in() + Argument ("response", "the output zonal spherical harmonic coefficients").type_file_out(); OPTIONS diff --git a/cpp/cmd/peaks2amp.cpp b/cpp/cmd/peaks2amp.cpp index e673453737..e009296efb 100644 --- a/cpp/cmd/peaks2amp.cpp +++ b/cpp/cmd/peaks2amp.cpp @@ -30,7 +30,7 @@ void usage() { SYNOPSIS = "Extract amplitudes from a peak directions image"; ARGUMENTS - + Argument ("directions", "the input directions image." + + Argument ("fibre_directions", "the input directions image." " Each volume corresponds to the x, y & z" " component of each direction vector in turn.").type_image_in() diff --git a/cpp/cmd/peaks2fixel.cpp b/cpp/cmd/peaks2fixel.cpp index 1b80c83fa1..a788920d53 100644 --- a/cpp/cmd/peaks2fixel.cpp +++ b/cpp/cmd/peaks2fixel.cpp @@ -34,7 +34,7 @@ void usage() { + Fixel::format_description; ARGUMENTS - + Argument ("directions", "the input directions image;" + + Argument ("fibre_directions", "the input directions image;" " each volume corresponds to the x, y & z" " component of each direction vector in turn.").type_image_in() diff --git a/cpp/cmd/sh2amp.cpp b/cpp/cmd/sh2amp.cpp index 15b30288fd..fae3f0758a 100644 --- a/cpp/cmd/sh2amp.cpp +++ b/cpp/cmd/sh2amp.cpp @@ -67,7 +67,7 @@ void usage() { ARGUMENTS + Argument ("input", "the input spherical harmonic (SH) coefficients image").type_image_in() - + Argument ("directions", "the set of directions along which the SH functions will be sampled").type_file_in() + + Argument ("fibre_directions", "the set of directions along which the SH functions will be sampled").type_file_in() + Argument ("output", "the output amplitudes image").type_image_out(); OPTIONS diff --git a/cpp/cmd/sh2response.cpp b/cpp/cmd/sh2response.cpp index 6a37182423..87e69b3668 100644 --- a/cpp/cmd/sh2response.cpp +++ b/cpp/cmd/sh2response.cpp @@ -42,7 +42,7 @@ void usage() { ARGUMENTS + Argument ("SH", "the spherical harmonic decomposition of the diffusion-weighted images").type_image_in() + Argument ("mask", "the mask containing the voxels from which to estimate the response function").type_image_in() - + Argument ("directions", "a 4D image containing the direction vectors along which to estimate the response function").type_image_in() + + Argument ("fibre_directions", "a 4D image containing the direction vectors along which to estimate the response function").type_image_in() + Argument ("response", "the output axially-symmetric spherical harmonic coefficients").type_file_out(); OPTIONS From 4447c509a47972bde45ce0660fbe6f659fb24810 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Wed, 27 Aug 2025 14:36:14 +1000 Subject: [PATCH 12/27] added pydra test github action --- .github/workflows/pydra.yml | 84 +++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 .github/workflows/pydra.yml diff --git a/.github/workflows/pydra.yml b/.github/workflows/pydra.yml new file mode 100644 index 0000000000..568a236f0d --- /dev/null +++ b/.github/workflows/pydra.yml @@ -0,0 +1,84 @@ +name: Checks + +on: + pull_request: + types: [opened, synchronize] + branches: [ master, dev] + merge_group: + types: [checks_requested] + branches: [master] + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + generate-tasks: + runs-on: ubuntu-latest + + env: + QT_SELECT: qt6 + SCCACHE_GHA_ENABLED: "true" + SCCACHE_CACHE_SIZE: "2G" + + steps: + - uses: actions/checkout@v1 + with: + submodules: true + + - name: Get latest version + run: echo "MRTRIX3_VERSION=$(git fetch --tags && git describe --tags --abbrev=0)" >> $GITHUB_ENV + + - name: install dependencies + run: | + sudo apt-get update + sudo apt-get install clang qt6-base-dev libglvnd-dev zlib1g-dev libfftw3-dev ninja-build python3-numpy libpng-dev + + - name: Run sccache-cache + uses: mozilla-actions/sccache-action@v0.0.9 + + - name: Get CMake + uses: lukka/get-cmake@latest + with: + cmakeVersion: '3.16.3' + + - name: Print CMake version + run: cmake --version + + - name: Make installation dir + run: | + sudo mkdir -p /opt/mrtrix3 + sudo chown $USER /opt/mrtrix3 + + - name: configure + run: > + cmake + -B build + -G Ninja + -D CMAKE_BUILD_TYPE=Release + -D MRTRIX_BUILD_TESTS=ON + -D MRTRIX_STL_DEBUGGING=ON + -D MRTRIX_WARNINGS_AS_ERRORS=ON + -D CMAKE_C_COMPILER=clang + -D CMAKE_CXX_COMPILER=clang++ + -D CMAKE_INSTALL_PREFIX=/opt/mrtrix3 + + - name: Build MRtrix3 + run: cmake --build build + + - name: Install MRtrix3 + run: cmake --install build + + - name: Clone pydra-tasks-mrtrix3 + run: git clone https://github.com/nipype/pydra-tasks-mrtrix3 + + - name: Install pydra-tasks-mrtrix3 and fileformats packages + run: pip install -e ./pydra-tasks-mrtrix3 -e ./pydra-tasks-mrtrix3/related-packages/fileformats -e ./pydra-tasks-mrtrix3/related-packages/fileformats-extras + + - name: Generate task packages + run: python3 pydra-tasks-mrtrix3/generate.py /opt/mrtrix3/bin pydra-tasks-mrtrix3 3.1.0 + + - name: Generate Pydra code + run: python3 -c "import pydra.tasks.mrtrix3.v3_1" + + From f88a4fd009ccdf33600e642f3cc9887a6431d7f6 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Thu, 28 Aug 2025 09:17:48 +1000 Subject: [PATCH 13/27] added dev option to pydra-tasks-mrtrix3 install --- .github/workflows/pydra.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pydra.yml b/.github/workflows/pydra.yml index 568a236f0d..6ae4ee5ed8 100644 --- a/.github/workflows/pydra.yml +++ b/.github/workflows/pydra.yml @@ -13,7 +13,7 @@ concurrency: cancel-in-progress: true jobs: - generate-tasks: + generate-pydra: runs-on: ubuntu-latest env: @@ -73,9 +73,9 @@ jobs: run: git clone https://github.com/nipype/pydra-tasks-mrtrix3 - name: Install pydra-tasks-mrtrix3 and fileformats packages - run: pip install -e ./pydra-tasks-mrtrix3 -e ./pydra-tasks-mrtrix3/related-packages/fileformats -e ./pydra-tasks-mrtrix3/related-packages/fileformats-extras + run: pip install -e ./pydra-tasks-mrtrix3[dev] -e ./pydra-tasks-mrtrix3/related-packages/fileformats -e ./pydra-tasks-mrtrix3/related-packages/fileformats-extras - - name: Generate task packages + - name: Generate Pydra task packages run: python3 pydra-tasks-mrtrix3/generate.py /opt/mrtrix3/bin pydra-tasks-mrtrix3 3.1.0 - name: Generate Pydra code From f78acd36f03bc7107101aca3ef4918cef6e70c4c Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Thu, 28 Aug 2025 09:59:27 +1000 Subject: [PATCH 14/27] updated reference docs to use new "fibre_directions" argument names --- docs/reference/commands/amp2response.rst | 2 +- docs/reference/commands/peaks2amp.rst | 2 +- docs/reference/commands/peaks2fixel.rst | 2 +- docs/reference/commands/sh2amp.rst | 2 +- docs/reference/commands/sh2response.rst | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/reference/commands/amp2response.rst b/docs/reference/commands/amp2response.rst index 67548381f0..cfef388007 100644 --- a/docs/reference/commands/amp2response.rst +++ b/docs/reference/commands/amp2response.rst @@ -13,7 +13,7 @@ Usage :: - amp2response [ options ] amps mask directions response + amp2response [ options ] amps mask fibre_directions response - *amps*: the amplitudes image - *mask*: the mask containing the voxels from which to estimate the response function diff --git a/docs/reference/commands/peaks2amp.rst b/docs/reference/commands/peaks2amp.rst index 9dd02a0465..29ae23b61f 100644 --- a/docs/reference/commands/peaks2amp.rst +++ b/docs/reference/commands/peaks2amp.rst @@ -13,7 +13,7 @@ Usage :: - peaks2amp [ options ] directions amplitudes + peaks2amp [ options ] fibre_directions amplitudes - *directions*: the input directions image. Each volume corresponds to the x, y & z component of each direction vector in turn. - *amplitudes*: the output amplitudes image. diff --git a/docs/reference/commands/peaks2fixel.rst b/docs/reference/commands/peaks2fixel.rst index 58a3274509..428a62e316 100644 --- a/docs/reference/commands/peaks2fixel.rst +++ b/docs/reference/commands/peaks2fixel.rst @@ -13,7 +13,7 @@ Usage :: - peaks2fixel [ options ] directions fixels + peaks2fixel [ options ] fibre_directions fixels - *directions*: the input directions image; each volume corresponds to the x, y & z component of each direction vector in turn. - *fixels*: the output fixel directory. diff --git a/docs/reference/commands/sh2amp.rst b/docs/reference/commands/sh2amp.rst index 6e0ecaa141..b60d98b782 100644 --- a/docs/reference/commands/sh2amp.rst +++ b/docs/reference/commands/sh2amp.rst @@ -13,7 +13,7 @@ Usage :: - sh2amp [ options ] input directions output + sh2amp [ options ] input fibre_directions output - *input*: the input spherical harmonic (SH) coefficients image - *directions*: the set of directions along which the SH functions will be sampled diff --git a/docs/reference/commands/sh2response.rst b/docs/reference/commands/sh2response.rst index e84c5f3fdd..2d0af84dcd 100644 --- a/docs/reference/commands/sh2response.rst +++ b/docs/reference/commands/sh2response.rst @@ -13,7 +13,7 @@ Usage :: - sh2response [ options ] SH mask directions response + sh2response [ options ] SH mask fibre_directions response - *SH*: the spherical harmonic decomposition of the diffusion-weighted images - *mask*: the mask containing the voxels from which to estimate the response function From 246e0621f4fbc7e9e628119c6ef05fad66e9b643 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Thu, 28 Aug 2025 10:22:10 +1000 Subject: [PATCH 15/27] fixing pylint errors in app.py --- python/mrtrix3/app.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/python/mrtrix3/app.py b/python/mrtrix3/app.py index a968c4438d..ba0b75c773 100644 --- a/python/mrtrix3/app.py +++ b/python/mrtrix3/app.py @@ -13,11 +13,9 @@ # # For more details, see http://www.mrtrix.org/. -import argparse, importlib, inspect, math, os, pathlib, random, shlex, shutil, signal, string, subprocess, sys, textwrap, time +import argparse, importlib, inspect, math, os, pathlib, random, shlex, shutil, signal, string, subprocess, sys, textwrap, time, re from mrtrix3 import ANSI, CONFIG, MRtrixError, setup_ansi from mrtrix3 import utils, version -import re -import typing as ty from keyword import kwlist as PYTHON_KEYWORDS try: import black.parsing From 3e0e84f5fd2a83b13c22ced023dec79bd24b6102 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Thu, 28 Aug 2025 10:25:23 +1000 Subject: [PATCH 16/27] fixing up pylint errors --- python/mrtrix3/app.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/python/mrtrix3/app.py b/python/mrtrix3/app.py index ba0b75c773..d4d0911beb 100644 --- a/python/mrtrix3/app.py +++ b/python/mrtrix3/app.py @@ -14,9 +14,9 @@ # For more details, see http://www.mrtrix.org/. import argparse, importlib, inspect, math, os, pathlib, random, shlex, shutil, signal, string, subprocess, sys, textwrap, time, re +from keyword import kwlist as PYTHON_KEYWORDS from mrtrix3 import ANSI, CONFIG, MRtrixError, setup_ansi from mrtrix3 import utils, version -from keyword import kwlist as PYTHON_KEYWORDS try: import black.parsing except ImportError: @@ -747,7 +747,7 @@ def _legacytypestring(): @staticmethod def _metavar(): return 'values' - + # Would you mind if this is defined here instead of in commands.population_template.usage # makes it much easiery to import and include in instance checks for pydra auto-gen?? @@ -760,7 +760,7 @@ def _legacytypestring(): return 'SEQDIROUT' @staticmethod def _metavar(): - return 'directory_list' + return 'directory_list' class DirectoryIn(CustomTypeBase): def __call__(self, input_value): @@ -1580,10 +1580,10 @@ def escape_id(id_: str) -> str: return escaped def is_output(arg_option) -> bool: - tp = arg_option.type - if isinstance(tp, (Parser.FileOut, Parser.DirectoryOut, Parser.ImageOut, Parser.SequenceDirectoryOut)): + typ = arg_option.type + if isinstance(typ, (Parser.FileOut, Parser.DirectoryOut, Parser.ImageOut, Parser.SequenceDirectoryOut)): return True - return hasattr(tp, "_legacytypestring") and tp._legacytypestring().endswith("OUT") + return hasattr(typ, "_legacytypestring") and typ._legacytypestring().endswith("OUT") inputs = [] outputs = [] From e84cf2dea409cc0611ec1c17941162dfcf932b90 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Thu, 28 Aug 2025 11:06:48 +1000 Subject: [PATCH 17/27] disable pylint too many public methods for app.Parser --- python/mrtrix3/app.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python/mrtrix3/app.py b/python/mrtrix3/app.py index d4d0911beb..aa65f03430 100644 --- a/python/mrtrix3/app.py +++ b/python/mrtrix3/app.py @@ -586,6 +586,7 @@ def _get_message(self): # MRtrix3 binaries, and defining functions for exporting the help page for the purpose of # automated self-documentation. +# pylint: disable=too-many-public-methods class Parser(argparse.ArgumentParser): # Function that will create a new class, From 91a62f6338fa036be2fb8f745cac38bf984f0f95 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Thu, 28 Aug 2025 11:07:38 +1000 Subject: [PATCH 18/27] renamed directions -> fibre_directions in ref docs --- docs/reference/commands/amp2response.rst | 2 +- docs/reference/commands/peaks2amp.rst | 2 +- docs/reference/commands/peaks2fixel.rst | 2 +- docs/reference/commands/sh2amp.rst | 2 +- docs/reference/commands/sh2response.rst | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/reference/commands/amp2response.rst b/docs/reference/commands/amp2response.rst index cfef388007..168bed3de8 100644 --- a/docs/reference/commands/amp2response.rst +++ b/docs/reference/commands/amp2response.rst @@ -17,7 +17,7 @@ Usage - *amps*: the amplitudes image - *mask*: the mask containing the voxels from which to estimate the response function -- *directions*: a 4D image containing the estimated fibre directions +- *fibre_directions*: a 4D image containing the estimated fibre directions - *response*: the output zonal spherical harmonic coefficients Description diff --git a/docs/reference/commands/peaks2amp.rst b/docs/reference/commands/peaks2amp.rst index 29ae23b61f..48f687945a 100644 --- a/docs/reference/commands/peaks2amp.rst +++ b/docs/reference/commands/peaks2amp.rst @@ -15,7 +15,7 @@ Usage peaks2amp [ options ] fibre_directions amplitudes -- *directions*: the input directions image. Each volume corresponds to the x, y & z component of each direction vector in turn. +- *fibre_directions*: the input directions image. Each volume corresponds to the x, y & z component of each direction vector in turn. - *amplitudes*: the output amplitudes image. Options diff --git a/docs/reference/commands/peaks2fixel.rst b/docs/reference/commands/peaks2fixel.rst index 428a62e316..2fdc239bd6 100644 --- a/docs/reference/commands/peaks2fixel.rst +++ b/docs/reference/commands/peaks2fixel.rst @@ -15,7 +15,7 @@ Usage peaks2fixel [ options ] fibre_directions fixels -- *directions*: the input directions image; each volume corresponds to the x, y & z component of each direction vector in turn. +- *fibre_directions*: the input directions image; each volume corresponds to the x, y & z component of each direction vector in turn. - *fixels*: the output fixel directory. Description diff --git a/docs/reference/commands/sh2amp.rst b/docs/reference/commands/sh2amp.rst index b60d98b782..f490cc04e6 100644 --- a/docs/reference/commands/sh2amp.rst +++ b/docs/reference/commands/sh2amp.rst @@ -16,7 +16,7 @@ Usage sh2amp [ options ] input fibre_directions output - *input*: the input spherical harmonic (SH) coefficients image -- *directions*: the set of directions along which the SH functions will be sampled +- *fibre_directions*: the set of directions along which the SH functions will be sampled - *output*: the output amplitudes image Description diff --git a/docs/reference/commands/sh2response.rst b/docs/reference/commands/sh2response.rst index 2d0af84dcd..3be7f06107 100644 --- a/docs/reference/commands/sh2response.rst +++ b/docs/reference/commands/sh2response.rst @@ -17,7 +17,7 @@ Usage - *SH*: the spherical harmonic decomposition of the diffusion-weighted images - *mask*: the mask containing the voxels from which to estimate the response function -- *directions*: a 4D image containing the direction vectors along which to estimate the response function +- *fibre_directions*: a 4D image containing the direction vectors along which to estimate the response function - *response*: the output axially-symmetric spherical harmonic coefficients Description From 9aa1bc7ff753af4875b6253b04f35ed2186afb3d Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Thu, 28 Aug 2025 11:16:53 +1000 Subject: [PATCH 19/27] run pytest instead of just importing pydra package in GH action --- .github/workflows/pydra.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pydra.yml b/.github/workflows/pydra.yml index 6ae4ee5ed8..b01c5f97f0 100644 --- a/.github/workflows/pydra.yml +++ b/.github/workflows/pydra.yml @@ -73,12 +73,13 @@ jobs: run: git clone https://github.com/nipype/pydra-tasks-mrtrix3 - name: Install pydra-tasks-mrtrix3 and fileformats packages - run: pip install -e ./pydra-tasks-mrtrix3[dev] -e ./pydra-tasks-mrtrix3/related-packages/fileformats -e ./pydra-tasks-mrtrix3/related-packages/fileformats-extras + run: pip install -e ./pydra-tasks-mrtrix3[test,dev] -e ./pydra-tasks-mrtrix3/related-packages/fileformats -e ./pydra-tasks-mrtrix3/related-packages/fileformats-extras - name: Generate Pydra task packages run: python3 pydra-tasks-mrtrix3/generate.py /opt/mrtrix3/bin pydra-tasks-mrtrix3 3.1.0 - - name: Generate Pydra code - run: python3 -c "import pydra.tasks.mrtrix3.v3_1" + - name: Test with pytest + run: pytest -sv pydra-tasks-mrtrix3/pydra/tasks/mrtrix3 --cov pydra.tasks.mrtrix3 --cov-report xml + From 9a455ea645db51590f23ceb53f3624eecce78ca9 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Fri, 28 Nov 2025 11:26:53 +1100 Subject: [PATCH 20/27] updated pydra code gen to match new argument-handling syntax --- cpp/core/app.cpp | 137 +++++++++++++++++++++++------------------------ 1 file changed, 66 insertions(+), 71 deletions(-) diff --git a/cpp/core/app.cpp b/cpp/core/app.cpp index c068b6d0c5..b80394769f 100644 --- a/cpp/core/app.cpp +++ b/cpp/core/app.cpp @@ -856,62 +856,56 @@ std::string pydra_code() { return escaped; }; - auto format_type = [&](const ArgType &type, bool optional = false) { - switch (type) { - case Undefined: - return "ty.Any"; - case Text: - return "str"; - case Boolean: - return "bool"; - case Integer: - return "int"; - case Float: - return "float"; - case ArgFileIn: - return "File"; - case ArgFileOut: - return "File"; - case ArgDirectoryIn: - return "Directory"; - case ArgDirectoryOut: - return "Directory"; - case Choice: - return "str"; - case ImageIn: - return "ImageIn"; - case ImageOut: - return "ImageOut"; - case IntSeq: - return "list[int]"; - case FloatSeq: - return "list[float]"; - case TracksIn: - return "Tracks"; - case TracksOut: - return "Tracks"; - case Various: - return "ty.Any"; - default: - assert(0); - } - return "not-reached"; + auto format_type = [&](const ArgTypeFlags &types, bool optional = false) { + std::string type; + if (types[ArgTypeFlags::Text]) + type += " | str"; + if (types[ArgTypeFlags::Boolean]) + type += " | bool"; + if (types[ArgTypeFlags::Integer]) + type += " | int"; + if (types[ArgTypeFlags::Float]) + type += " | float"; + if (types[ArgTypeFlags::FileIn]) + type += " | File"; + if (types[ArgTypeFlags::FileOut]) + type += " | File"; + if (types[ArgTypeFlags::DirectoryIn]) + type += " | Directory"; + if (types[ArgTypeFlags::DirectoryOut]) + type += " | Directory"; + if (types[ArgTypeFlags::Choice]) + type += " | str"; + if (types[ArgTypeFlags::ImageIn]) + type += " | ImageIn"; + if (types[ArgTypeFlags::ImageOut]) + type += " | ImageOut"; + if (types[ArgTypeFlags::IntSeq]) + type += " | list[int]"; + if (types[ArgTypeFlags::FloatSeq]) + type += " | list[float]"; + if (types[ArgTypeFlags::TracksIn]) + type += " | Tracks"; + if (types[ArgTypeFlags::TracksOut]) + type += " | Tracks"; + assert(type); + return type.substr(3); // drop the preceding " | " }; auto format_option_type = [&](const Option &opt, bool for_output = false) { std::string f; - bool is_multi = (opt.flags & AllowMultiple) && (!opt.size() || opt[0].type != ArgFileOut); + bool is_multi = (opt.flags.allow_multiple()) && (!opt.size() || !opt[0].types[ArgTypeFlags::FileOut]); if (is_multi) { f += "MultiInputObj["; } if (!opt.size()) { f += "bool"; } else if (opt.size() == 1) { - f += format_type(opt[0].type, true); + f += format_type(opt[0].types, true); } else { f += "tuple["; for (size_t a = 0; a < opt.size(); ++a) { - f += format_type(opt[0].type, true); + f += format_type(opt[0].types, true); if (a != opt.size() - 1) { f += ", "; } @@ -926,22 +920,21 @@ std::string pydra_code() { auto format_choices = [&](const Argument &arg) { std::string f = indent + "allowed_values=["; - std::vector choices = std::get>(arg.limits); - f += "\"" + choices[0] + "\""; - for (int i = 1; i < choices.size(); ++i) { - f += ", \"" + choices[i] + "\""; + f += "\"" + arg.choices[0] + "\""; + for (int i = 1; i < arg.choices.size(); ++i) { + f += ", \"" + arg.choices[i] + "\""; } f += "],\n"; return f; }; - auto format_output_template = [&](const std::string &id, const ArgType &type) { + auto format_output_template = [&](const std::string &id, const ArgTypeFlags &types) { std::string tmpl(id); - if (type == ImageOut) { + if (types[ArgTypeFlags::ImageOut]) { tmpl += ".mif"; - } else if (type == TracksOut) { + } else if (types[ArgTypeFlags::TracksOut]) { tmpl += ".tck"; - } else if (type == ArgFileOut) { + } else if (types[ArgTypeFlags::FileOut]) { tmpl += ".txt"; } // TODO: Add special cases for file-out based on the 'id' where the extension @@ -951,10 +944,10 @@ std::string pydra_code() { auto format_output_templates = [&](const std::string &id, const Option &opt) { if (opt.size() == 1) - return "\"" + format_output_template(id, opt[0].type) + "\""; + return "\"" + format_output_template(id, opt[0].types) + "\""; std::string tmpl = "("; for (size_t i = 0; i < opt.size(); ++i) - tmpl += "\"" + format_output_template(id + MR::str(i), opt[i].type) + "\","; + tmpl += "\"" + format_output_template(id + MR::str(i), opt[i].types) + "\","; tmpl += ")"; return tmpl; }; @@ -962,13 +955,13 @@ std::string pydra_code() { auto format_arg_name = [&](const Argument &arg) { std::string id = arg.id; std::string arg_name; - if (id == "input" && (arg.type == ImageIn || arg.type == ArgFileIn)) + if (id == "input" && (arg.types[ArgTypeFlags::ImageIn] || arg.types[ArgTypeFlags::FileIn])) arg_name = "in_file"; - else if (id == "input" && arg.type == ArgDirectoryIn) + else if (id == "input" && arg.types[ArgTypeFlags::DirectoryIn]) arg_name = "in_dir"; - else if (id == "output" && (arg.type == ImageOut || arg.type == ArgFileOut)) + else if (id == "output" && (arg.types[ArgTypeFlags::ImageOut] || arg.types[ArgTypeFlags::FileOut])) arg_name = "out_file"; - else if (id == "output" && arg.type == ArgDirectoryOut) + else if (id == "output" && arg.types[ArgTypeFlags::DirectoryOut]) arg_name = "out_dir"; else arg_name = escape_id(arg.id); @@ -976,7 +969,7 @@ std::string pydra_code() { }; auto format_argument = [&](const Argument &arg, int position, bool is_output = false) { - bool is_multi = (arg.flags & AllowMultiple) && (arg.type != ArgFileOut); + bool is_multi = (arg.flags.allow_multiple()) && (!arg.types[ArgTypeFlags::FileOut]); // Print name of field std::string f = ""; std::string arg_name = format_arg_name(arg); @@ -989,11 +982,11 @@ std::string pydra_code() { else type += "MultiInputObj["; } - type += format_type(arg.type); + type += format_type(arg.types); if (is_multi) { type += "]"; } - if (arg.flags & Optional) { + if (arg.flags.optional()) { type += " | None"; } f += type; @@ -1004,19 +997,19 @@ std::string pydra_code() { // Print metadata fields f += indent + "argstr=\"\",\n"; f += indent + "position=" + std::to_string(position) + ",\n"; - if (arg.flags & Optional) { - if (arg.type == Boolean) + if (arg.flags.optional()) { + if (arg.types[ArgTypeFlags::Boolean]) f += indent + "default=False,\n"; else f += indent + "default=None,\n"; } if (is_output) { - f += indent + "path_template=\"" + format_output_template(arg_name, arg.type) + "\",\n"; + f += indent + "path_template=\"" + format_output_template(arg_name, arg.types) + "\",\n"; } f += indent + "help=\"\"\"" + arg.desc + "\"\"\",\n"; - if (arg.type == Choice) { + if (arg.types[ArgTypeFlags::Choice]) { f += format_choices(arg); } f += base_indent + ")\n"; @@ -1030,7 +1023,7 @@ std::string pydra_code() { bool is_multi = type_string.length() > 19 && type_string.substr(0, 19) == "MultiInputObj"; if (is_output && !is_multi) { type_string += "| bool | None"; - } else if (opt.flags & Optional && type_string != "bool" && type_string != "ty.Any") { + } else if (opt.flags.optional() && type_string != "bool" && type_string != "ty.Any") { type_string += " | None"; } // Print type @@ -1039,7 +1032,7 @@ std::string pydra_code() { f += " = shell.outarg(\n"; else f += " = shell.arg(\n"; - if (opt.flags & Optional) { + if (opt.flags.optional()) { if (type_string == "bool") f += indent + "default=False,\n"; else @@ -1051,12 +1044,12 @@ std::string pydra_code() { f += indent + "path_template=" + format_output_templates(escape_id(opt.id), opt) + ",\n"; } f += indent + "help=\"\"\"" + opt.desc + "\"\"\",\n"; - if (opt.size() == 1 && (opt[0].type == IntSeq || opt[0].type == FloatSeq)) { + if (opt.size() == 1 && (opt[0].types[ArgTypeFlags::IntSeq] || opt[0].types[ArgTypeFlags::FloatSeq])) { f += indent + "sep=\",\",\n"; } else if (opt.size() > 1) { f += indent + "sep=\" \",\n"; } - if (opt.size() == 1 && opt[0].type == Choice) { + if (opt.size() == 1 && opt[0].types[ArgTypeFlags::Choice]) { f += format_choices(opt[0]); } f += base_indent + ")\n"; @@ -1065,14 +1058,16 @@ std::string pydra_code() { auto option_is_output = [&](const Option &opt) { for (size_t i = 0; i < opt.size(); ++i) { - if (opt[i].type == ImageOut || opt[i].type == ArgFileOut || opt[i].type == ArgDirectoryOut) + if (opt[i].types[ArgTypeFlags::ImageOut] || opt[i].types[ArgTypeFlags::FileOut] || + opt[i].types[ArgTypeFlags::DirectoryOut]) return true; } return false; }; auto argument_is_output = [&](const Argument &arg) { - return arg.type == ImageOut || arg.type == ArgFileOut || arg.type == ArgDirectoryOut; + return arg.types[ArgTypeFlags::ImageOut] || arg.types[ArgTypeFlags::FileOut] || + arg.types[ArgTypeFlags::DirectoryOut]; }; // Create actual class @@ -1098,7 +1093,7 @@ std::string pydra_code() { s += "\n" + base_indent + "References\n" + base_indent + "----------\n\n"; for (size_t i = 0; i < REFERENCES.size(); ++i) s += indent + REFERENCES[i] + "\n\n"; - s += indent + MRTRIX_CORE_REFERENCE + "\n\n"; + s += indent + core_reference + "\n\n"; s += "\n" + base_indent + "MRtrix\n" + base_indent + "------" + "\n\n" + base_indent + "Version:" + mrtrix_version + ", built " + build_date + "\n\n" + base_indent + "Author: " + AUTHOR + "\n\n" + base_indent + From 23fff5b89c9a64ae15dbe21fee35ad368b4fd89c Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Fri, 28 Nov 2025 12:57:31 +1100 Subject: [PATCH 21/27] debugging generation of pydra code after arg parsing refactor merge --- cpp/core/app.cpp | 9 ++++++--- python/mrtrix3/app.py | 8 ++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/cpp/core/app.cpp b/cpp/core/app.cpp index b80394769f..c537e0e232 100644 --- a/cpp/core/app.cpp +++ b/cpp/core/app.cpp @@ -837,7 +837,7 @@ std::string pydra_code() { s += "import typing as ty \n"; s += "from pathlib import Path # noqa: F401\n"; s += "from fileformats.generic import File, Directory # noqa: F401\n"; - s += "from fileformats.medimage_mrtrix3 import ImageIn, ImageOut, Tracks # noqa: F401\n"; + s += "from fileformats.vendor.mrtrix3.medimage import ImageIn, ImageOut, Tracks # noqa: F401\n"; s += "from pydra.compose import shell\n"; s += "from pydra.utils.typing import MultiInputObj\n"; @@ -888,8 +888,11 @@ std::string pydra_code() { type += " | Tracks"; if (types[ArgTypeFlags::TracksOut]) type += " | Tracks"; - assert(type); - return type.substr(3); // drop the preceding " | " + if (type.empty()) + type = "ty.Any"; + else + type = type.substr(3); // drop the preceding " | " + return type; }; auto format_option_type = [&](const Option &opt, bool for_output = false) { diff --git a/python/mrtrix3/app.py b/python/mrtrix3/app.py index fc71177a55..f30459f356 100644 --- a/python/mrtrix3/app.py +++ b/python/mrtrix3/app.py @@ -15,6 +15,7 @@ import argparse, importlib, inspect, math, os, pathlib, random, shlex, shutil, signal, string, subprocess, sys, textwrap, time, re from keyword import kwlist as PYTHON_KEYWORDS +from warnings import warn from mrtrix3 import ANSI, CONFIG, MRtrixError, setup_ansi from mrtrix3 import utils, version try: @@ -1521,8 +1522,6 @@ def get_arg_metadata(arg): def parse_type(type_, optional: bool = False): if type_ is str or type_ is None: type_str = "str" - elif isinstance(type_, Parser.Various): - type_str = "typing.Any" elif isinstance(type_, Parser.Bool): type_str = "bool" elif type(type_).__name__ == "IntBounded": @@ -1552,7 +1551,8 @@ def parse_type(type_, optional: bool = False): elif isinstance(type_, Parser.TracksOut): type_str = "Tracks" else: - raise ValueError("Unrecognized type: " + str(type_)) + warn("Unrecognized type: " + str(type_) + " defaulting to ty.Any") + type_str = "ty.Any" if optional: type_str += " | None" return type_str @@ -1690,7 +1690,7 @@ def cmd_to_task_name(cmd_name: str) -> str: "import typing\n" "from pathlib import Path # noqa: F401\n" "from fileformats.generic import FsObject, File, Directory # noqa: F401\n" - "from fileformats.medimage_mrtrix3 import Tracks, ImageIn, ImageOut # noqa: F401\n" + "from fileformats.vendor.mrtrix3.medimage import Tracks, ImageIn, ImageOut # noqa: F401\n" "from pydra.utils.typing import MultiInputObj\n" "from pydra.compose import shell\n" ) From 3024e1190fb1b31ab7f268b498b8d22d5eb505e6 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Fri, 28 Nov 2025 16:03:17 +1100 Subject: [PATCH 22/27] fixed up code to pass syntax check --- cpp/core/app.cpp | 22 +++++++++++----------- python/mrtrix3/app.py | 12 ++++++------ 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/cpp/core/app.cpp b/cpp/core/app.cpp index c537e0e232..16f33b0c28 100644 --- a/cpp/core/app.cpp +++ b/cpp/core/app.cpp @@ -788,7 +788,7 @@ std::string pydra_code() { "Fod", "Label", "Mask", "Mesh", "Mr", "Mt", "Peaks", "Sh", "Tck", "Tensor", "Transform", "Tsf", "Voxel", "Vector", "Warp"}; - auto convert_to_pascal_case = [&](const std::string &input) { + auto convert_to_pascal_case = [&](const std::string_view input) { std::string result; bool capitalizeNext = true; for (char c : input) { @@ -802,7 +802,7 @@ std::string pydra_code() { result += c; if (c == '2') capitalizeNext = true; - for (const std::string &prefix : CMD_PREFIXES) { + for (const std::string_view prefix : CMD_PREFIXES) { if (result == prefix) { capitalizeNext = true; break; @@ -834,22 +834,22 @@ std::string pydra_code() { // Add import lines std::string s = std::string("# Auto-generated from MRtrix C++ command with '__print_pydra_code__' secret option\n\n"); - s += "import typing as ty \n"; + s += "from typing import Any \n"; s += "from pathlib import Path # noqa: F401\n"; s += "from fileformats.generic import File, Directory # noqa: F401\n"; s += "from fileformats.vendor.mrtrix3.medimage import ImageIn, ImageOut, Tracks # noqa: F401\n"; s += "from pydra.compose import shell\n"; s += "from pydra.utils.typing import MultiInputObj\n"; - auto escape_id = [&](const std::string &id) { - std::string escaped = id; + auto escape_id = [&](const std::string_view id) { + std::string escaped(id); // Replace any spaces and periods with underscores std::replace(escaped.begin(), escaped.end(), ' ', '_'); std::replace(escaped.begin(), escaped.end(), '.', '_'); // Append any Python keywords with an underscore bool is_keyword = std::any_of(std::begin(PYTHON_KEYWORDS), std::end(PYTHON_KEYWORDS), - [&id](const std::string &kword) { return kword == id; }); + [&id](const std::string_view kword) { return kword == id; }); if (is_keyword) escaped += "_"; @@ -889,7 +889,7 @@ std::string pydra_code() { if (types[ArgTypeFlags::TracksOut]) type += " | Tracks"; if (type.empty()) - type = "ty.Any"; + type = "Any"; else type = type.substr(3); // drop the preceding " | " return type; @@ -931,7 +931,7 @@ std::string pydra_code() { return f; }; - auto format_output_template = [&](const std::string &id, const ArgTypeFlags &types) { + auto format_output_template = [&](const std::string_view id, const ArgTypeFlags &types) { std::string tmpl(id); if (types[ArgTypeFlags::ImageOut]) { tmpl += ".mif"; @@ -945,12 +945,12 @@ std::string pydra_code() { return tmpl; }; - auto format_output_templates = [&](const std::string &id, const Option &opt) { + auto format_output_templates = [&](const std::string_view id, const Option &opt) { if (opt.size() == 1) return "\"" + format_output_template(id, opt[0].types) + "\""; std::string tmpl = "("; for (size_t i = 0; i < opt.size(); ++i) - tmpl += "\"" + format_output_template(id + MR::str(i), opt[i].types) + "\","; + tmpl += "\"" + format_output_template(std::string(id) + MR::str(i), opt[i].types) + "\","; tmpl += ")"; return tmpl; }; @@ -1026,7 +1026,7 @@ std::string pydra_code() { bool is_multi = type_string.length() > 19 && type_string.substr(0, 19) == "MultiInputObj"; if (is_output && !is_multi) { type_string += "| bool | None"; - } else if (opt.flags.optional() && type_string != "bool" && type_string != "ty.Any") { + } else if (opt.flags.optional() && type_string != "bool" && type_string != "Any") { type_string += " | None"; } // Print type diff --git a/python/mrtrix3/app.py b/python/mrtrix3/app.py index f30459f356..f898c0760a 100644 --- a/python/mrtrix3/app.py +++ b/python/mrtrix3/app.py @@ -1537,22 +1537,22 @@ def parse_type(type_, optional: bool = False): elif isinstance(type_, Parser.DirectoryOut): type_str = "Directory" elif isinstance(type_, Parser.SequenceDirectoryOut): - type_str = "typing.List[Directory]" + type_str = "list[Directory]" elif isinstance(type_, Parser.ImageIn): type_str = "ImageIn" elif isinstance(type_, Parser.ImageOut): type_str = "ImageOut" elif isinstance(type_, Parser.SequenceInt): - type_str = "typing.List[int]" + type_str = "list[int]" elif isinstance(type_, Parser.SequenceFloat): - type_str = "typing.List[float]" + type_str = "list[float]" elif isinstance(type_, Parser.TracksIn): type_str = "Tracks" elif isinstance(type_, Parser.TracksOut): type_str = "Tracks" else: - warn("Unrecognized type: " + str(type_) + " defaulting to ty.Any") - type_str = "ty.Any" + warn("Unrecognized type: " + str(type_) + " defaulting to Any") + type_str = "Any" if optional: type_str += " | None" return type_str @@ -1687,7 +1687,7 @@ def cmd_to_task_name(cmd_name: str) -> str: text = ( "# Auto-generated by mrtrix3/app.py:print_pydra_code()\n\n" - "import typing\n" + "from typing import Any\n" "from pathlib import Path # noqa: F401\n" "from fileformats.generic import FsObject, File, Directory # noqa: F401\n" "from fileformats.vendor.mrtrix3.medimage import Tracks, ImageIn, ImageOut # noqa: F401\n" From f4e3bf5f400281fe561363dfb4d2b62a01912b60 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Fri, 28 Nov 2025 16:10:09 +1100 Subject: [PATCH 23/27] cleaned up app.py import --- python/mrtrix3/app.py | 1 - 1 file changed, 1 deletion(-) diff --git a/python/mrtrix3/app.py b/python/mrtrix3/app.py index f898c0760a..88b7b9d41c 100644 --- a/python/mrtrix3/app.py +++ b/python/mrtrix3/app.py @@ -15,7 +15,6 @@ import argparse, importlib, inspect, math, os, pathlib, random, shlex, shutil, signal, string, subprocess, sys, textwrap, time, re from keyword import kwlist as PYTHON_KEYWORDS -from warnings import warn from mrtrix3 import ANSI, CONFIG, MRtrixError, setup_ansi from mrtrix3 import utils, version try: From d2dfdfb631540775c4a4a1b45691b9a0cc1fc7d3 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Fri, 28 Nov 2025 16:42:23 +1100 Subject: [PATCH 24/27] Renamed pydra CI workflow --- .github/workflows/pydra.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pydra.yml b/.github/workflows/pydra.yml index b01c5f97f0..38470967b4 100644 --- a/.github/workflows/pydra.yml +++ b/.github/workflows/pydra.yml @@ -1,4 +1,4 @@ -name: Checks +name: Pydra on: pull_request: From cb6bc45c11342a57d51766bf8cfe156ef2845db8 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Fri, 28 Nov 2025 17:01:05 +1100 Subject: [PATCH 25/27] renamed pydra CI workflow --- .github/workflows/pydra.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pydra.yml b/.github/workflows/pydra.yml index 38470967b4..74423cdccf 100644 --- a/.github/workflows/pydra.yml +++ b/.github/workflows/pydra.yml @@ -1,4 +1,4 @@ -name: Pydra +name: Pydra Code Generation on: pull_request: From 24a09ac515db0e26004f6b7cca0abf16af52c96f Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Mon, 1 Dec 2025 11:49:03 +1100 Subject: [PATCH 26/27] Stylistic changes in response to code-review --- .github/workflows/pydra.yml | 2 +- cpp/core/app.cpp | 24 +++++++++++++----------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/.github/workflows/pydra.yml b/.github/workflows/pydra.yml index 74423cdccf..8d76d7f107 100644 --- a/.github/workflows/pydra.yml +++ b/.github/workflows/pydra.yml @@ -1,4 +1,4 @@ -name: Pydra Code Generation +name: Pydra code generation on: pull_request: diff --git a/cpp/core/app.cpp b/cpp/core/app.cpp index 16f33b0c28..761fa82c55 100644 --- a/cpp/core/app.cpp +++ b/cpp/core/app.cpp @@ -784,9 +784,9 @@ std::string restructured_text_usage() { std::string pydra_code() { - std::string CMD_PREFIXES[] = {"Fivett", "Afd", "Amp", "Connectome", "Dcm", "Dir", "Dwi", "Fixel", - "Fod", "Label", "Mask", "Mesh", "Mr", "Mt", "Peaks", "Sh", - "Tck", "Tensor", "Transform", "Tsf", "Voxel", "Vector", "Warp"}; + constexpr std::array CMD_PREFIXES = { + "Fivett", "Afd", "Amp", "Connectome", "Dcm", "Dir", "Dwi", "Fixel", "Fod", "Label", "Mask", "Mesh", + "Mr", "Mt", "Peaks", "Sh", "Tck", "Tensor", "Transform", "Tsf", "Voxel", "Vector", "Warp"}; auto convert_to_pascal_case = [&](const std::string_view input) { std::string result; @@ -826,7 +826,7 @@ std::string pydra_code() { std::string base_indent(" "); std::string indent = base_indent + " "; - std::string PYTHON_KEYWORDS[] = { + constexpr std::array PYTHON_KEYWORDS = { "and", "as", "assert", "break", "class", "continue", "def", "del", "elif", "else", "except", "False", "finally", "for", "from", "global", "if", "import", "in", "is", "lambda", "None", "nonlocal", "not", "or", "pass", "raise", @@ -889,19 +889,21 @@ std::string pydra_code() { if (types[ArgTypeFlags::TracksOut]) type += " | Tracks"; if (type.empty()) - type = "Any"; + type = "str"; else type = type.substr(3); // drop the preceding " | " + if (optional) + type += " | None"; return type; }; - auto format_option_type = [&](const Option &opt, bool for_output = false) { + auto format_option_type = [&](const Option &opt) { std::string f; bool is_multi = (opt.flags.allow_multiple()) && (!opt.size() || !opt[0].types[ArgTypeFlags::FileOut]); if (is_multi) { f += "MultiInputObj["; } - if (!opt.size()) { + if (opt.empty()) { f += "bool"; } else if (opt.size() == 1) { f += format_type(opt[0].types, true); @@ -1077,12 +1079,12 @@ std::string pydra_code() { s += "\n\n@shell.define\nclass " + name_string + "(shell.Task[\"" + name_string + ".Outputs\"]):\n"; s += " \"\"\""; // Add description - if (DESCRIPTION.size()) { + if (!DESCRIPTION.empty()) { for (size_t i = 0; i < DESCRIPTION.size(); ++i) s += base_indent + std::string(DESCRIPTION[i]) + "\n\n"; } - if (EXAMPLES.size()) { + if (!EXAMPLES.empty()) { s += "\n" + base_indent + "Example usages\n" + base_indent + "--------------\n\n"; for (size_t i = 0; i < EXAMPLES.size(); ++i) { s += "\n" + base_indent + EXAMPLES[i].title + ":\n\n"; @@ -1326,11 +1328,11 @@ void parse() { } if (num_optional_arguments && num_args_required > argument.size()) - throw Exception("Expected at least " + str(num_args_required) + " arguments (" + str(argument.size()) + + throw Exception("Expected at least " + str(num_args_required) + " arguments (" + str(!argument.empty()) + " supplied)"); if (num_optional_arguments == 0 && num_args_required != argument.size()) { - Exception e("Expected exactly " + str(num_args_required) + " arguments (" + str(argument.size()) + " supplied)"); + Exception e("Expected exactly " + str(num_args_required) + " arguments (" + str(!argument.empty()) + " supplied)"); std::string s = "Usage: " + NAME; for (const auto &a : ARGUMENTS) s += " " + std::string(a.id); From 08d9393edd7e993eb417a1eef908262994703f62 Mon Sep 17 00:00:00 2001 From: "Thomas G. Close" Date: Wed, 3 Dec 2025 10:21:02 +1100 Subject: [PATCH 27/27] reverted use of optional kwarg in pydra code-gen type formatting --- cpp/core/app.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/cpp/core/app.cpp b/cpp/core/app.cpp index 761fa82c55..9f849f7b76 100644 --- a/cpp/core/app.cpp +++ b/cpp/core/app.cpp @@ -856,7 +856,7 @@ std::string pydra_code() { return escaped; }; - auto format_type = [&](const ArgTypeFlags &types, bool optional = false) { + auto format_type = [&](const ArgTypeFlags &types) { std::string type; if (types[ArgTypeFlags::Text]) type += " | str"; @@ -892,8 +892,6 @@ std::string pydra_code() { type = "str"; else type = type.substr(3); // drop the preceding " | " - if (optional) - type += " | None"; return type; }; @@ -906,11 +904,11 @@ std::string pydra_code() { if (opt.empty()) { f += "bool"; } else if (opt.size() == 1) { - f += format_type(opt[0].types, true); + f += format_type(opt[0].types); } else { f += "tuple["; for (size_t a = 0; a < opt.size(); ++a) { - f += format_type(opt[0].types, true); + f += format_type(opt[0].types); if (a != opt.size() - 1) { f += ", "; }