Skip to content
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

♻️ Split ResourcePool into three classes #2

Merged
merged 77 commits into from
Aug 9, 2024

Conversation

shnizzedy
Copy link

(This PR is FCP-INDI#2131 with a different target branch)

Description

Separates https://github.com/FCP-INDI/C-PAC/blob/5ce5d379f8de00ca73032d7beafa28c29f792668/CPAC/pipeline/engine.py#L69-L1413 into three classes: Resource, ResourcePool, and StratPool.

Technical details

flowchart LR
subgraph "(before)"
subgraph "cpac.pipeline (before)"
subgraph "cpac_pipeline"
list_blocks.before(list_blocks)
end
subgraph "engine (before)"
initiate_rpool
NodeData
run_node_blocks
set_iterables
strip_template
subgraph "NodeBlock (before)"
NodeBlockFunction.before(NodeBlockFunction)
nodeblock.before(nodeblock)
NodeBlock.__init__.before(__init__)
NodeBlock.get_name(get_name)
check_output.before(check_output)
check_null
NodeBlock.connect_block(connect_block)
grab_tiered_dct
end
subgraph "ResourcePool (before)"
ResourcePool.__init__.before(__init__)
ResourcePool.append_name(append_name)
back_propogate_template_name.before(back_propogate_template_name)
check_rpool.before(check_rpool)
copy_resource
copy_rpool
derivative_xfm
filter_name
filtered_movement
flatten_prov
gather_pipes
generate_prov_list
get.before(get)
get_cpac_provenance
get_data.before(get_data)
get_entire_rpool
get_json
get_json_info.before(get_json_info)
get_name
get_pipe_idxs
get_pipe_number
get_pool_info
set_pool_info
get_raw_label.before(get_raw_label)
get_resource_from_prov
get_resource_strats_from_prov
get_resources
get_strat_info
get_strats
node_data
post_process
regressor_dct.before(regressor_dct)
set_data.before(set_data)
update_resource
end
wrap_block
ingress_raw_anat_data
ingress_freesurfer
ingress_raw_func_data
ingress_output_dir
json_outdir_ingress
func_outdir_ingress
ingress_pipeconfig_paths
end
end
end
subgraph "(after)"
subgraph cpac.pipeline.engine
subgraph nodeblock
nodeblock.after(nodeblock)
NodeBlockFunction
subgraph NodeBlock
NodeBlock.__init__(__init__)
check_output
list_blocks
end
end
subgraph resource
ResourceData
_check_null
resource.set_iterables(set_iterables)
resource.strip_template(strip_template)
subgraph Resource
Resource.get_json(get_json)
data
json
keys
cpac_provenance
end
subgraph _Pool
_Pool.__init__(__init__)
_Pool.check_rpool(check_rpool)
_Pool.get(get)
_Pool.get_resource_from_prov(get_resource_from_prov)
_Pool.keys(keys)
set_data
subgraph ResourcePool
ResourcePool.__init__.after(__init__)
back_propogate_template_name.after(back_propogate_template_name)
ResourcePool.connect_block(connect_block)
ResourcePool.derivative_xfm(derivative_xfm)
ResourcePool.gather_pipes(gather_pipes)
ResourcePool.get(get)
ResourcePool.get_data(get_data)
ResourcePool.get_json(get_json)
get_json_info.after(get_json_info)
_get_pipe_number
get_raw_label.after(get_raw_label)
ResourcePool.get_resource_strats_from_prov(get_resource_strats_from_prov)
ResourcePool.get_strats(get_strats)
ResourcePool.ingress_raw_anat_data(ingress_raw_anat_data)
ResourcePool.ingress_freesurfer(ingress_freesurfer)
ResourcePool.ingress_raw_func_data(ingress_raw_func_data)
ResourcePool.ingress_output_dir(ingress_output_dir)
ResourcePool.json_outdir_ingress(json_outdir_ingress)
ResourcePool.func_outdir_ingress(func_outdir_ingress)
ResourcePool.post_process(post_process)
ResourcePool.ingress_pipeconfig_paths(ingress_pipeconfig_paths)
end
subgraph StratPool
StratPool.__init__(__init__)
StratPool.append_name(append_name)
StratPool.copy_resource(copy_resource)
StratPool.filter_name(filter_name)
StratPool.filtered_movement(filtered_movement)
StratPool.get(get)
StratPool.get_cpac_provenance(get_cpac_provenance)
StratPool.get_data(get_data)
StratPool.get_json(get_json)
regressor_dct
end
end
end
end
end
ResourcePool.__init__.before --> _Pool.__init__
_Pool.__init__ -.-> ResourcePool.__init__.after
_Pool.__init__ -.-> StratPool.__init__
back_propogate_template_name.before --> back_propogate_template_name.after
ResourcePool.append_name --> StratPool.append_name
check_rpool.before --> _Pool.check_rpool
copy_resource --> StratPool.copy_resource
derivative_xfm --> ResourcePool.derivative_xfm
filter_name --> StratPool.filter_name
filtered_movement --> StratPool.filtered_movement
gather_pipes --> ResourcePool.gather_pipes(gather_pipes)
get_cpac_provenance --> StratPool.get_cpac_provenance
get_data.before --> ResourcePool.get_data
get_data.before --> StratPool.get_data
get_json --> Resource.get_json
get_json --> ResourcePool.get_json
get_json --> StratPool.get_json
get_json_info.before --> get_json_info.after
get_pipe_number --> _get_pipe_number
get_resources --> _Pool.keys
get_raw_label.before --> get_raw_label.after
get_resource_strats_from_prov --> ResourcePool.get_resource_strats_from_prov
get_resource_from_prov --> _Pool.get_resource_from_prov
get_strats --> ResourcePool.get_strats
node_data --> DELETED
post_process --> ResourcePool.post_process
regressor_dct.before --> regressor_dct
set_data.before --> set_data
copy_rpool --> DELETED
get.before --> _Pool.get
_Pool.get -.-> ResourcePool.get
_Pool.get -.-> StratPool.get
get_name --> DELETED("🗑️")
flatten_prov --> DELETED
generate_prov_list --> DELETED
get_pipe_idxs --> DELETED
get_pool_info --> DELETED
set_pool_info --> DELETED
get_entire_rpool --> DELETED
get_strat_info --> DELETED
update_resource --> DELETED
NodeBlock.__init__.before --> NodeBlock.__init__
NodeBlock.get_name --> DELETED
check_null --> _check_null
grab_tiered_dct --> DELETED
NodeBlock.connect_block --> ResourcePool.connect_block
wrap_block --> DELETED
ingress_raw_anat_data --> ResourcePool.ingress_raw_anat_data
ingress_freesurfer --> ResourcePool.ingress_freesurfer(ingress_freesurfer)
ingress_raw_func_data --> ResourcePool.ingress_raw_func_data(ingress_raw_func_data)
ingress_output_dir --> ResourcePool.ingress_output_dir(ingress_output_dir)
json_outdir_ingress --> ResourcePool.json_outdir_ingress(json_outdir_ingress)
func_outdir_ingress --> ResourcePool.func_outdir_ingress(func_outdir_ingress)
set_iterables --> resource.set_iterables
strip_template --> resource.strip_template
initiate_rpool --> ResourcePool.__init__.after
run_node_blocks --> DELETED
NodeData --> ResourceData
NodeBlockFunction.before --> NodeBlockFunction
nodeblock.before --> nodeblock.after
check_output.before --> check_output
list_blocks.before --> list_blocks
Loading

Tests

I converted these tests to pytest tests and turned them back on: https://github.com/FCP-INDI/C-PAC/blob/41f15ce50de05dca71a92dc96bf86df0c59bc83c/CPAC/pipeline/test/test_engine.py#L45-L101

Screenshots

Checklist

  • My pull request has a descriptive title (not a vague title like Update index.md).
  • My pull request targets the develop branch of the repository.
  • My commit messages follow best practices.
  • My code follows the established code style of the repository.
  • I updated tests for the changes I made.
  • I updated the changelog.
  • I added or updated documentation (📝 Document "♻️ Split ResourcePool into three classes" FCP-INDI/fcp-indi.github.io#325).
  • I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

shnizzedy added 30 commits July 8, 2024 17:17
…ces`

Squashed commit of the following:

commit a40c449
Merge: 6b6f7f4 9ba084e
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Mon Jul 15 13:16:14 2024 -0400

    ⬆️ Bump zipp from 3.16.0 to 3.19.1 (#2132)

commit 9ba084e
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Tue Jul 9 18:35:58 2024 +0000

    :arrow_up: Bump zipp from 3.16.0 to 3.19.1

    Bumps [zipp](https://github.com/jaraco/zipp) from 3.16.0 to 3.19.1.
    - [Release notes](https://github.com/jaraco/zipp/releases)
    - [Changelog](https://github.com/jaraco/zipp/blob/main/NEWS.rst)
    - [Commits](jaraco/zipp@v3.16.0...v3.19.1)

    ---
    updated-dependencies:
    - dependency-name: zipp
      dependency-type: direct:production
    ...

    Signed-off-by: dependabot[bot] <support@github.com>

commit 6b6f7f4
Merge: 5ce5d37 979b0a9
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Fri Jul 12 13:27:18 2024 -0400

    🐛 Fix `get_scan_params` (#2129)

commit 979b0a9
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Tue Jul 9 09:39:03 2024 -0400

    :pencil2: Fix f-string missing `f`

commit af65a2e
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Mon Jul 8 17:01:06 2024 -0400

    :bug: Fix circular import

commit f194377
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Mon Jul 8 16:07:15 2024 -0400

    :recycle: Exclusively use custom `Function` Nodes + :rotating_light: Lint

commit c7819d1
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Mon Jul 8 14:16:53 2024 -0400

    :art: Remove unnecessary initializations

commit b013ccc
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Mon Jul 8 13:54:48 2024 -0400

    :package: Init `Function`

commit b19907a
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Mon Jul 8 13:40:19 2024 -0400

    :bug: Use C-PAC Function node

commit 7d6f0ee
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Mon Jul 8 12:59:12 2024 -0400

    :pencil2: Fix TR capitalization

commit 3ebb9f4
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Mon Jul 8 12:54:48 2024 -0400

    :white_check_mark: Add tests for `fetch` refactor

    [rebuild base-lite]
    [rebuild base-standard]
    [run reg-suite]

commit 6a5b723
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Mon Jul 8 12:01:02 2024 -0400

    :recycle: DRY params, sub, scan

commit c5c39b0
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Mon Jul 8 10:49:11 2024 -0400

    :bug: Tell Nipype to import typehint type

    [run reg-suite]

commit 52aa366
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Mon Jul 8 10:05:09 2024 -0400

    :recycle: DRY `fetch_and_convert`|`fetch`|`check`|`check2`

    [run reg-suite]

commit ddf2103
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Sat Jul 6 00:27:06 2024 -0400

    :construction: WIP :bug: Fix `get_scan_params`

    [run reg-suite]

commit 17257e3
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Fri Jul 5 14:26:27 2024 -0400

    :recycle: Dedupe function node imports

commit 74c0950
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Fri Jul 5 09:56:33 2024 -0400

    :bug: Import local variables in `get_scan_params`

    [run reg-suite]

commit e64309c
Author: Jon Clucas <jon.clucas@childmind.org>
Date:   Wed Jul 3 18:15:43 2024 -0400

    :bug: Fix import (probably merge error)

    [run reg-suite]
@sgiavasis
Copy link
Owner

Great. Thanks for the rebase. Merging this in first as it is more congruent with the codebase as-is.

@sgiavasis sgiavasis merged commit 532322d into sgiavasis:develop Aug 9, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants