-
Notifications
You must be signed in to change notification settings - Fork 180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Implement Pydra code-generators #2665
Draft
tclose
wants to merge
116
commits into
MRtrix3:dev
Choose a base branch
from
tclose:pydra-usage
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+791
−4
Draft
Changes from all commits
Commits
Show all changes
116 commits
Select commit
Hold shift + click to select a range
057980c
implemented print_pydra_usage for C++ commands
tclose c8568b0
touch up imports in pydra_usage_print output
tclose ea474a5
implementing type-aware output_file_template
tclose cae2e23
fixed up output_file_template
tclose e5c56f8
added return statement to make gcc happy
tclose 1451c06
implemented python __print_usage_pydra__ option
tclose 3152260
cleaned up pylint issues in app.py
tclose 35fbece
debugged handling of type printing in python __print_usage_pydra__
tclose 192e30c
started to add in pydra-package to be auto-generated as nested direct…
tclose 446874e
preparing pydra directory for auto-genned code
tclose b024c8f
fixed up gitignore
tclose 20f2f91
fixed handling of choices
tclose 4d50e12
removed debugging statements from code gen
tclose b59ab55
debugged remaining issues with python pydra export
tclose dc0a90e
escaped 5tt* cmd names with "filetissuetype" prefix
tclose 68ac1be
renamed auto-gen script
tclose a405041
added DS_store to gitignore
tclose 445e3aa
configured pydra pyproject.toml
tclose a3ebfc3
replaced 5tt with five_tissue_type in pydra module names
tclose 7cb21a0
added github action to build pydra package
tclose 33146fd
fixed error in gh action
tclose 373d38a
expanded pydra github action to trigger on pull requests to dev
tclose b8d4c11
added requirements file
tclose 2679b07
fixed up auto-gen call in GHA
tclose 548d400
added mrtrix build to pydra deploy GHA
tclose 441a76e
reinstated checkout actions in other pydra GHA steps
tclose 517da78
added log-errors option to pydra-auto-gen.py script
tclose 448fb70
added separate licence file for pydra
tclose d43d056
added separate licence for pydra, really this time
tclose d7467cf
added artifacts to GHA build/deploy workflow
tclose d5c6f6f
fixed up shared lib upload artifact in GHA
tclose c14b63a
removed upload of build script
tclose 7c90b55
renamed pydra GHA
tclose f646cab
set path and ld_library_path in gha
tclose 31927b9
escaped all help_strings in triple quotes in pydra auto gen
tclose a19f79b
set execute perms on bin dir of pydra GHA
tclose aa12a94
combined build and generate into single step
tclose 7c56b98
added flake8 opts to pydra package toml
tclose 7549961
added support for pydra usage for python cmd with algorithm
tclose 6f6eca8
added return statement after generating pydra tasks for python algori…
tclose 5a8b312
reverted debugging statements
tclose 183b5cb
handle the case of nested commands i.e. algorithms
tclose 33511d4
reverted auto formatting in algorithm.py
tclose 88f6520
escape python keywords in pydra print in c++ cmds
tclose f7501fe
bug fixes to pydra-usage printing: escape python keyword args and add…
tclose 6e50b6c
added pydra handling for options with multiple file args
tclose 5e122ea
added test generation code to pydra generate
tclose 105e215
escaped cmd_name
tclose 128cc8d
added fileformats-mrtrix package
tclose de66aed
added MGH and MGZ to supported file-formats
tclose 0a34415
added in mrtrix formats into fileformats-mrtrix
tclose 7d6a08d
updated Mgh format name
tclose faba88d
renamed TrackFile format to Tracks
tclose 563ba6c
removed mistakenly staged generated file
tclose 452cb10
fixed up dwi fileformats
tclose f18870f
debugging automatic test generation
tclose b71e6e5
added auto-generated tests
tclose 311e9c7
added in fileformats-mrtrix3-extras package
tclose e3d1812
touched up fileformats extras readme
tclose 3c24809
added in more fileformats-mrtrix3 extras
tclose 10855bc
fixing up fileformats packages before initial release
tclose 4fe6902
added in mrconvert converter to fileformats-mrtrix3
tclose ac855f3
getting fileformats-extras tests to pass
tclose b77e06b
added in_file/out_file kwargs to converter
tclose 7d0cab0
split converters so they aren't wrapped twice
tclose 82b3b4b
Pydra gen: added task name to input and output specs to aid debugging
tclose fbcae8e
debugged pydra autotest generation
tclose 54d2cb1
debugged pydra untittest structure
tclose f2dd3ec
renamed directions argument so it doesn't clash with 'directions' option
tclose 6b587f3
renamed fileformats-mrtrix to fileformats-medimage-mrtrix
tclose fb20ade
renamed fileformats.mrtrix package name
tclose 7ba8adf
renamed fileformats.mrtrix package name in extras package
tclose 107ffbc
renamed pydra-auto-gen.py to generate.py
tclose d8444a8
changed import path for fileformats mrtrix3
tclose 5e607b6
renamed fileformats.mrtrix3 to fileformats.medimage_mrtrix3
tclose a8a7436
added some pydra-specific keywords to PYTHON_KEYWORDS
tclose 9510ee0
added back in pydra_usage function to app.cpp
tclose 4b5ee0d
added __print_usage_pydra__ special option back in
tclose ebe7af8
removed generated _version files
tclose ab787d2
updated dicom import location
tclose 511b549
updated Mgz format name
tclose 8674e91
added mif.gz format
tclose 3a28637
touched up pydra test fixtures to set the right paths and env vars
tclose 7e4db06
added comment to start of generated pydra commands from pythong
tclose ec75006
debugging unittests
tclose dad98ca
updated generated sample data args
tclose 37fc50b
updated syntax to latest version of fileformats
tclose 252fd72
fixed up extras signatures
tclose 09a8f1a
added separate b-file formats for MIF/MIH
tclose ad9cf92
added support for multi-line mif header values
tclose 333d88d
added manually constructed mrcalc interface
tclose 673522d
added alias to Op in mrcalc operators
tclose 7283d7b
imported gzipped image formats into base fileformats package
1908468
converted command names to pascal case
tclose 0ee5a4d
removed generated pydra files from git
tclose a6dfbee
moved pydra src inside 'interfaces' directory
tclose 3350178
added _version.py to gitignore
tclose 86271ff
removed stray .DS_Store
tclose ea3d590
replace 'input' and 'output' fields with "in_file" and "out_file" to …
tclose 8adb7ff
added pytest_cache to gitignore
tclose 9f41b6a
removed generated pydra files from git repo
tclose 3fecc1f
touch up
tclose 147993b
updating pydra package github action
tclose 76d6bd8
touched up pydra GH action workflow
tclose d1c2ddd
debugging pydra GH action
tclose 617d9b2
fix up
tclose 6bfef76
log generation errors instead of raising them to test rest of deploym…
tclose 8f90927
touched up logging in generation script
tclose 520bcb1
fix up GH action
tclose 834da49
fix up
tclose b5d2802
delete stray generated files
tclose 2072799
removed more stray files
tclose 5c30f14
stripped back pydra GH action to be testing only
tclose 098361b
deleted pydra package stub in favour of storing it in nipype/pydra-mr…
tclose 0ff5bd1
removed flake8 config
tclose b24273f
Merge branch 'dev' into pydra-usage
tclose File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,173 @@ | ||
#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: | ||
|
||
build: | ||
|
||
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/[email protected] | ||
|
||
- 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: Clone Pydra-MRtrix3 package | ||
run: git clone https://github.com/nipype/pydra-mrtrix3.git pydra | ||
|
||
- name: Install pydra-auto-gen requirements | ||
run: | | ||
pip install -e ./pydra/fileformats-medimage-mrtrix3 | ||
pip install -e ./pydra/fileformats-medimage-mrtrix3-extras | ||
pip install -e ./pydra[dev] | ||
|
||
- name: Generate task specifications | ||
run: > | ||
./pydra/generate.py | ||
$(pwd)/install/bin | ||
$(pwd)/pydra/src | ||
$MRTRIX3_VERSION | ||
--log-errors | ||
--latest | ||
|
||
- name: Upload MRtrix3 install | ||
uses: actions/upload-artifact@v2 | ||
with: | ||
name: MRtrix3 | ||
path: mrtrix3/install | ||
|
||
- name: Upload auto-gen pydra | ||
uses: actions/upload-artifact@v2 | ||
with: | ||
name: AutoGen | ||
path: pydra/tasks/mrtrix3/$MRTRIX3_VERSION | ||
|
||
- name: Write version file | ||
run: echo $MRTRIX3_VERSION > mrtrix3_version.txt | ||
|
||
- name: Upload version file | ||
uses: actions/upload-artifact@v2 | ||
with: | ||
name: VersionFile | ||
path: pydra/tasks/mrtrix3/$MRTRIX3_VERSION | ||
|
||
test: | ||
needs: [build] | ||
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
python-version: [3.8, 3.11] | ||
defaults: | ||
run: | ||
shell: bash -l {0} | ||
steps: | ||
|
||
- name: Download version file | ||
uses: actions/download-artifact@v2 | ||
with: | ||
name: VersionFile | ||
path: mrtrix3_version.txt | ||
|
||
- name: Extract Mrtrix version | ||
run: echo "MRTRIX3_VERSION=$(cat mrtrix3_version.txt)" >> $GITHUB_ENV | ||
|
||
- 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 pydra/fileformats-medimage-mrtrix pydra/fileformats-medimage-mrtrix-extras '.[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 pydra/tasks/mrtrix3 \ | ||
--cov pydra.tasks.mrtrix3 --cov-report xml | ||
- uses: codecov/codecov-action@v1 | ||
if: ${{ always() }} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this to avoid a clash with an option of the same name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Hid / resolved prior post about the same issue)
Issue here is that the argument "
directions
" and the command-line option "directions
" encode two very different pieces of information; so disambiguating by calling one an "image" may not be the optimal change. I'm hesitant to change the command-line option, as there are many commands relating to "amp
" images that have an option "-directions
" (even if it's not currently centralised with aconst Option
incore/
/src/
). Conversely, the functionality of the positional argument here also appears in other commands; so renaming it to something different only here would also be slightly unusual.A few possibilities come to mind:
Across various commands (thinking
dwi2response
in particular), where an image encoding a unit 3-vector per voxel is used, adopt "orientations
" rather than "directions
".While it's technically applicable to the data type in terms of command naming convention, "
peaks
" would likely not be a good choice here, since can't have multiple vectors per voxel, and the data may have been derived using some approach other than ODF peak-finding.Rename here (and preferably in other comparable instances also) to "
fibre_directions
".Create a function that returns an
Option
, creating the current "-directions
" command-line option with a custom help string per use (I'm pretty sure from experience it's the considerable difference in these help strings that has caused there to not be centralisation of this command-line option). Once this works, change the command-line option name to something like "-amp_directions
".(We might need to think carefully about our use of the header field
"Directions"
also...)